From 2ddd5df7dc950aa8f7dbea9498f63e97ab50b315 Mon Sep 17 00:00:00 2001
From: William O'Beirne <wbobeirne@gmail.com>
Date: Mon, 16 Apr 2018 19:07:00 -0400
Subject: [PATCH] Fix Ledger DPaths, Improve Error Messaging (#1517)

---
 .../components/DeterministicWalletsModal.tsx  | 22 ++++++----
 .../WalletDecrypt/components/LedgerNano.tsx   | 42 +++++++++++++++----
 common/config/dpaths.ts                       |  4 +-
 common/libs/validators.ts                     | 12 +++++-
 .../config/networks/staticNetworks.ts         |  9 ++--
 common/sass/styles/overrides/alerts.scss      | 22 ----------
 common/translations/lang/en.json              |  4 ++
 7 files changed, 68 insertions(+), 47 deletions(-)

diff --git a/common/components/WalletDecrypt/components/DeterministicWalletsModal.tsx b/common/components/WalletDecrypt/components/DeterministicWalletsModal.tsx
index 7ae8ab73..c12d6c78 100644
--- a/common/components/WalletDecrypt/components/DeterministicWalletsModal.tsx
+++ b/common/components/WalletDecrypt/components/DeterministicWalletsModal.tsx
@@ -199,15 +199,19 @@ class DeterministicWalletsModalClass extends React.PureComponent<Props, State> {
   private getAddresses(props: Props = this.props) {
     const { dPath, publicKey, chainCode, seed } = props;
 
-    if (dPath && ((publicKey && chainCode) || seed) && isValidPath(dPath)) {
-      this.props.getDeterministicWallets({
-        seed,
-        dPath,
-        publicKey,
-        chainCode,
-        limit: WALLETS_PER_PAGE,
-        offset: WALLETS_PER_PAGE * this.state.page
-      });
+    if (dPath && ((publicKey && chainCode) || seed)) {
+      if (isValidPath(dPath)) {
+        this.props.getDeterministicWallets({
+          seed,
+          dPath,
+          publicKey,
+          chainCode,
+          limit: WALLETS_PER_PAGE,
+          offset: WALLETS_PER_PAGE * this.state.page
+        });
+      } else {
+        console.error('Invalid dPath provided', dPath);
+      }
     }
   }
 
diff --git a/common/components/WalletDecrypt/components/LedgerNano.tsx b/common/components/WalletDecrypt/components/LedgerNano.tsx
index a46383d9..06c36e0c 100644
--- a/common/components/WalletDecrypt/components/LedgerNano.tsx
+++ b/common/components/WalletDecrypt/components/LedgerNano.tsx
@@ -9,6 +9,8 @@ import { connect } from 'react-redux';
 import { AppState } from 'reducers';
 import { SecureWalletName, ledgerReferralURL } from 'config';
 import { getPaths, getSingleDPath } from 'selectors/config/wallet';
+import { getNetworkConfig } from 'selectors/config';
+import { NetworkConfig } from 'types/network';
 import './LedgerNano.scss';
 
 interface OwnProps {
@@ -18,6 +20,7 @@ interface OwnProps {
 interface StateProps {
   dPath: DPath | undefined;
   dPaths: DPath[];
+  network: NetworkConfig;
 }
 
 interface State {
@@ -54,6 +57,7 @@ class LedgerNanoSDecryptClass extends PureComponent<Props, State> {
   }
 
   public render() {
+    const { network } = this.props;
     const { dPath, publicKey, chainCode, error, isLoading, showTip } = this.state;
     const showErr = error ? 'is-showing' : '';
 
@@ -74,12 +78,6 @@ class LedgerNanoSDecryptClass extends PureComponent<Props, State> {
 
     return (
       <div className="LedgerDecrypt">
-        {showTip && (
-          <p>
-            <strong>Tip: </strong>Make sure you're logged into the ethereum app on your hardware
-            wallet
-          </p>
-        )}
         <button
           className="LedgerDecrypt-decrypt btn btn-primary btn-lg btn-block"
           onClick={this.handleNullConnect}
@@ -101,6 +99,10 @@ class LedgerNanoSDecryptClass extends PureComponent<Props, State> {
 
         <div className={`LedgerDecrypt-error alert alert-danger ${showErr}`}>{error || '-'}</div>
 
+        {showTip && (
+          <p className="LedgerDecrypt-tip">{translate('LEDGER_TIP', { $network: network.unit })}</p>
+        )}
+
         <div className="LedgerDecrypt-help">
           <NewTabLink href="https://support.ledgerwallet.com/hc/en-us/articles/115005200009">
             {translate('HELP_ARTICLE_1')}
@@ -143,13 +145,34 @@ class LedgerNanoSDecryptClass extends PureComponent<Props, State> {
           });
         })
         .catch((err: any) => {
+          let showTip;
+          let errMsg;
+          // Timeout
           if (err && err.metaData && err.metaData.code === 5) {
-            this.showTip();
+            showTip = true;
+            errMsg = translateRaw('LEDGER_TIMEOUT');
           }
+          // Wrong app logged into
+          if (err && err.includes && err.includes('6804')) {
+            showTip = true;
+            errMsg = translateRaw('LEDGER_WRONG_APP');
+          }
+          // Ledger locked
+          if (err && err.includes && err.includes('6801')) {
+            errMsg = translateRaw('LEDGER_LOCKED');
+          }
+          // Other
+          if (!errMsg) {
+            errMsg = err && err.metaData ? err.metaData.type : err.toString();
+          }
+
           this.setState({
-            error: err && err.metaData ? err.metaData.type : err.toString(),
+            error: errMsg,
             isLoading: false
           });
+          if (showTip) {
+            this.showTip();
+          }
         });
     });
   };
@@ -179,7 +202,8 @@ class LedgerNanoSDecryptClass extends PureComponent<Props, State> {
 function mapStateToProps(state: AppState): StateProps {
   return {
     dPath: getSingleDPath(state, SecureWalletName.LEDGER_NANO_S),
-    dPaths: getPaths(state, SecureWalletName.LEDGER_NANO_S)
+    dPaths: getPaths(state, SecureWalletName.LEDGER_NANO_S),
+    network: getNetworkConfig(state)
   };
 }
 
diff --git a/common/config/dpaths.ts b/common/config/dpaths.ts
index f9e6da2b..dd3b0720 100644
--- a/common/config/dpaths.ts
+++ b/common/config/dpaths.ts
@@ -15,7 +15,7 @@ export const ETH_LEDGER: DPath = {
 
 export const ETC_LEDGER: DPath = {
   label: 'Ledger (ETC)',
-  value: "m/44'/60'/160720'/0"
+  value: "m/44'/60'/160720'/0'"
 };
 
 export const ETC_TREZOR: DPath = {
@@ -86,5 +86,5 @@ export const EXTRA_PATHS = [ETH_SINGULAR];
 
 // whitespace strings are evaluated the same way as nospace strings, except they allow optional spaces between each portion of the string
 // ie. "m / 44' / 0' / 0'" is valid, "m / 4 4' / 0' / 0'" is invalid
-export const dPathRegex = /m\/(44|0)'\/[0-9]+\'\/[0-9]+(\'+$|\'+(\/[0-1]+$))/;
+export const dPathRegex = /m\/44'\/[0-9]+\'\/[0-9]+(\'+$|\'+(\/[0-1]+$))/;
 // export const whitespaceDPathRegex = /m\s*\/\s*44'\s*\/\s*[0-9]+\'\s*\/\s*[0-9]+(\'+$|\'+\s*(\/\s*[0-1]+$))/;
diff --git a/common/libs/validators.ts b/common/libs/validators.ts
index c970139d..e64be598 100644
--- a/common/libs/validators.ts
+++ b/common/libs/validators.ts
@@ -11,7 +11,7 @@ import {
   GAS_PRICE_GWEI_LOWER_BOUND,
   GAS_PRICE_GWEI_UPPER_BOUND
 } from 'config/constants';
-import { dPathRegex } from 'config/dpaths';
+import { dPathRegex, ETC_LEDGER, ETH_SINGULAR } from 'config/dpaths';
 import { EAC_SCHEDULING_CONFIG } from './scheduling';
 import BN from 'bn.js';
 
@@ -126,6 +126,16 @@ export function isPositiveIntegerOrZero(num: number): boolean {
 }
 
 export function isValidPath(dPath: string) {
+  // ETC Ledger is incorrect up due to an extra ' at the end of it
+  if (dPath === ETC_LEDGER.value) {
+    return true;
+  }
+
+  // SingularDTV is incorrect due to using a 0 instead of a 44 as the purpose
+  if (dPath === ETH_SINGULAR.value) {
+    return true;
+  }
+
   return dPathRegex.test(dPath);
 }
 
diff --git a/common/reducers/config/networks/staticNetworks.ts b/common/reducers/config/networks/staticNetworks.ts
index 315e0e67..492a057a 100644
--- a/common/reducers/config/networks/staticNetworks.ts
+++ b/common/reducers/config/networks/staticNetworks.ts
@@ -68,7 +68,7 @@ export const INITIAL_STATE: State = {
     isTestnet: true,
     dPathFormats: {
       [SecureWalletName.TREZOR]: ETH_TESTNET,
-      [SecureWalletName.LEDGER_NANO_S]: ETH_TESTNET,
+      [SecureWalletName.LEDGER_NANO_S]: ETH_LEDGER,
       [InsecureWalletName.MNEMONIC_PHRASE]: ETH_TESTNET
     },
     gasPriceSettings: testnetDefaultGasPrice
@@ -88,7 +88,7 @@ export const INITIAL_STATE: State = {
     isTestnet: true,
     dPathFormats: {
       [SecureWalletName.TREZOR]: ETH_TESTNET,
-      [SecureWalletName.LEDGER_NANO_S]: ETH_TESTNET,
+      [SecureWalletName.LEDGER_NANO_S]: ETH_LEDGER,
       [InsecureWalletName.MNEMONIC_PHRASE]: ETH_TESTNET
     },
     gasPriceSettings: testnetDefaultGasPrice
@@ -108,7 +108,7 @@ export const INITIAL_STATE: State = {
     isTestnet: true,
     dPathFormats: {
       [SecureWalletName.TREZOR]: ETH_TESTNET,
-      [SecureWalletName.LEDGER_NANO_S]: ETH_TESTNET,
+      [SecureWalletName.LEDGER_NANO_S]: ETH_LEDGER,
       [InsecureWalletName.MNEMONIC_PHRASE]: ETH_TESTNET
     },
     gasPriceSettings: testnetDefaultGasPrice
@@ -199,7 +199,7 @@ export const INITIAL_STATE: State = {
     contracts: [],
     dPathFormats: {
       [SecureWalletName.TREZOR]: POA_DEFAULT,
-      [SecureWalletName.LEDGER_NANO_S]: POA_DEFAULT,
+      [SecureWalletName.LEDGER_NANO_S]: ETH_LEDGER,
       [InsecureWalletName.MNEMONIC_PHRASE]: POA_DEFAULT
     },
     gasPriceSettings: {
@@ -221,6 +221,7 @@ export const INITIAL_STATE: State = {
     tokens: [],
     contracts: [],
     dPathFormats: {
+      [SecureWalletName.LEDGER_NANO_S]: ETH_LEDGER,
       [SecureWalletName.TREZOR]: ETH_TREZOR,
       [SecureWalletName.LEDGER_NANO_S]: TOMO_DEFAULT,
       [InsecureWalletName.MNEMONIC_PHRASE]: TOMO_DEFAULT
diff --git a/common/sass/styles/overrides/alerts.scss b/common/sass/styles/overrides/alerts.scss
index 4ce8690a..b2820333 100644
--- a/common/sass/styles/overrides/alerts.scss
+++ b/common/sass/styles/overrides/alerts.scss
@@ -25,28 +25,6 @@
   }
 }
 
-// Alert icons
-.alert:after {
-  content: '';
-  background-position: 50%;
-  background-repeat: no-repeat;
-  background-size: contain;
-  display: block;
-  color: #fff;
-  position: absolute;
-  top: 0;
-  bottom: 0;
-  left: 1%;
-  width: $space * 2;
-
-  @media screen and (max-width: $screen-sm) {
-    left: 3%;
-  }
-  @media screen and (max-width: $screen-xs) {
-    left: 1%;
-  }
-}
-
 .alert,
 .alert-info {
   @include alert-variant($alert-info-bg, $alert-info-border, $alert-info-text);
diff --git a/common/translations/lang/en.json b/common/translations/lang/en.json
index 06b1d4b2..aaf79f4d 100644
--- a/common/translations/lang/en.json
+++ b/common/translations/lang/en.json
@@ -384,6 +384,10 @@
     "ETHER_ADDRESS_LOOKUP": "Ether Address Lookup",
     "LEDGER_REFERRAL_1": "Buy a Ledger Wallet",
     "LEDGER_REFERRAL_2": "Don’t have a Ledger? Order one now!",
+    "LEDGER_TIP": "**Tip:** Make sure you're logged into the $network app on your hardware wallet, and have enabled browser support in the settings",
+    "LEDGER_TIMEOUT": "The request timed out",
+    "LEDGER_WRONG_APP": "Wrong application selected on your device",
+    "LEDGER_LOCKED": "Your Ledger device is locked",
     "TREZOR_REFERAL": "Buy a TREZOR",
     "ETHERCARD_REFERAL": "Get an ether.card",
     "DISCLAIMER": "Disclaimer",