Fix custom derivation path (#4658)

* Fix alignment of warning icon when Input is of size m * Remove custom derivation path from redux because it should not be persistent. Add error message when derivation path is invalid * Prevent the currently typed passphrase from being deleted when changing enableCustomDerivationPath setting * Remove test because customDerivationPath is not in redux store anymore * Use the defaultDerivationPath when enableCustomDerivationPath is true * Remove test for login component which we will deprecate * remove unused * Add tests for CustomDerivationPath * Use getByLabelText from render instead of importing it, also change the test name to correspond to what is does * Add test to make sure that onAddAccount is called with both a passphrase and a derivationPath * Add test for when passphrase is correct but derivation path is incorrect. * Fix typo * Remove this second failsafe check since the button is already disabled this function cannot be called if isSubmitDisabled, add test for checking if button is disabled instead * Make sure disabled buttons dont have cursor pointer * remove unused css rule * add link to issue
LiskHQ · Dec 12, 2022 · 06f6084 · 06f6084
1 parent 01f498f
commit 06f6084
Show file tree

Hide file tree

Showing 14 changed files with 190 additions and 74 deletions.
diff --git a/src/locales/en/common.json b/src/locales/en/common.json
@@ -293,6 +293,7 @@
   "Invalid address or public key": "Invalid address or public key",
   "Invalid amount": "Invalid amount",
   "Invalid dates": "Invalid dates",
+  "Invalid path format": "Invalid path format",
   "Keep it safe as it is the only way to access your wallet.": "Keep it safe as it is the only way to access your wallet.",
   "Keep up-to-date with announcements from the Lisk Foundation. Check what network delegates have been up to with dedicated profile pages.": "Keep up-to-date with announcements from the Lisk Foundation. Check what network delegates have been up to with dedicated profile pages.",
   "LSK deposited": "LSK deposited",

diff --git a/src/modules/account/components/AddAccountBySecretRecovery/AddAccountBySecretRecovery.js b/src/modules/account/components/AddAccountBySecretRecovery/AddAccountBySecretRecovery.js
@@ -13,10 +13,12 @@ import styles from './AddAccountBySecretRecovery.css';
 const AddAccountBySecretRecovery = ({ history }) => {
   const multiStepRef = useRef(null);
   const [recoveryPhrase, setRecoveryPhrase] = useState(null);
+  const [customDerivationPath, setCustomDerivationPath] = useState();
   const [currentAccount, setCurrentAccount] = useCurrentAccount();
   const { setAccount } = useAccounts();
-  const onAddAccount = (recoveryPhraseData) => {
+  const onAddAccount = (recoveryPhraseData, derivationPath) => {
     setRecoveryPhrase(recoveryPhraseData);
+    setCustomDerivationPath(derivationPath);
     multiStepRef.current.next();
   };
 
@@ -37,7 +39,7 @@ const AddAccountBySecretRecovery = ({ history }) => {
         ref={multiStepRef}
       >
         <AddAccountForm onAddAccount={onAddAccount} />
-        <SetPasswordForm recoveryPhrase={recoveryPhrase} onSubmit={onSetPassword} />
+        <SetPasswordForm recoveryPhrase={recoveryPhrase} customDerivationPath={customDerivationPath} onSubmit={onSetPassword} />
         <SetPasswordSuccess
           encryptedPhrase={currentAccount}
           onClose={onPasswordSetComplete}

diff --git a/src/modules/account/components/AddAccountForm/AddAccountForm.js b/src/modules/account/components/AddAccountForm/AddAccountForm.js
@@ -1,5 +1,5 @@
 /* eslint-disable max-lines */
-import React, { useState } from 'react';
+import React, { useMemo, useState } from 'react';
 import { useTranslation } from 'react-i18next';
 import grid from 'flexboxgrid/dist/flexboxgrid.css';
 import { Link } from 'react-router-dom';
@@ -9,10 +9,11 @@ import { PrimaryButton } from 'src/theme/buttons';
 import PassphraseInput from 'src/modules/wallet/components/PassphraseInput/PassphraseInput';
 import DiscreetModeToggle from 'src/modules/settings/components/discreetModeToggle';
 import NetworkSelector from 'src/modules/settings/components/networkSelector';
+import { getDerivationPathErrorMessage } from '@wallet/utils/account';
+import { defaultDerivationPath } from 'src/utils/explicitBipKeyDerivation';
 import styles from './AddAccountForm.css';
 
 const AddAccountForm = ({ settings, onAddAccount }) => {
-  const { t } = useTranslation();
   const [passphrase, setPass] = useState({ value: '', isValid: false });
 
   const setPassphrase = (value, error) => {
@@ -22,18 +23,42 @@ const AddAccountForm = ({ settings, onAddAccount }) => {
     });
   };
 
+  const props = { settings, onAddAccount, setPassphrase, passphrase };
+
+  if (settings.enableCustomDerivationPath) {
+    return <AddAccountFormWithDerivationPath {...props} />;
+  }
+  return <AddAccountFormContainer {...props} />;
+};
+
+const AddAccountFormContainer = ({
+  settings,
+  passphrase,
+  setPassphrase,
+  onAddAccount,
+  isSubmitDisabled,
+  derivationPath,
+  children,
+}) => {
+  const { t } = useTranslation();
+
   const onFormSubmit = (e) => {
     e.preventDefault();
     // istanbul ignore else
     if (passphrase.value && passphrase.isValid) {
-      onAddAccount(passphrase);
+      onAddAccount(passphrase, derivationPath);
     }
   };
 
   const handleKeyPress = (e) => {
     if (e.charCode === 13) onFormSubmit(e);
   };
 
+  const passphraseArray = useMemo(
+    () => passphrase.value?.replace(/\W+/g, ' ')?.split(/\s/),
+    [passphrase.value]
+  );
+
   return (
     <div className={`${styles.addAccount}`}>
       <div className={`${styles.wrapper} ${grid['col-xs-12']}`}>
@@ -52,20 +77,21 @@ const AddAccountForm = ({ settings, onAddAccount }) => {
             <fieldset>
               <label>{t('Secret recovery phrase')}</label>
               <PassphraseInput
-                inputsLength={12}
+                inputsLength={passphraseArray?.length > 12 ? 24 : 12}
                 maxInputsLength={24}
                 onFill={setPassphrase}
                 keyPress={handleKeyPress}
+                values={passphraseArray}
               />
             </fieldset>
-            <CustomDerivationPath />
+            {children}
             <DiscreetModeToggle className={styles.discreetMode} />
           </div>
           <div className={`${styles.buttonsHolder}`}>
             <PrimaryButton
               className={`${styles.button} login-button`}
               type="submit"
-              disabled={!passphrase.isValid}
+              disabled={!passphrase.isValid || isSubmitDisabled}
             >
               {t('Continue')}
             </PrimaryButton>
@@ -82,4 +108,30 @@ const AddAccountForm = ({ settings, onAddAccount }) => {
   );
 };
 
+const AddAccountFormWithDerivationPath = (props) => {
+  const [derivationPath, setDerivationPath] = useState(defaultDerivationPath);
+  const derivationPathErrorMessage = useMemo(
+    () => getDerivationPathErrorMessage(derivationPath),
+    [derivationPath]
+  );
+
+  const onDerivationPathChange = (value) => {
+    setDerivationPath(value);
+  };
+
+  return (
+    <AddAccountFormContainer
+      {...props}
+      isSubmitDisabled={!!derivationPathErrorMessage}
+      derivationPath={derivationPath}
+    >
+      <CustomDerivationPath
+        onChange={onDerivationPathChange}
+        value={derivationPath}
+        errorMessage={derivationPathErrorMessage}
+      />
+    </AddAccountFormContainer>
+  );
+};
+
 export default AddAccountForm;
diff --git a/src/modules/account/components/AddAccountForm/AddAccountForm.test.js b/src/modules/account/components/AddAccountForm/AddAccountForm.test.js
@@ -20,7 +20,7 @@ beforeEach(() => {
   });
 });
 
-describe('Generals', () => {
+describe('AddAccountForm', () => {
   it('should render successfully', () => {
     expect(screen.getByText('Add account')).toBeTruthy();
     expect(
@@ -85,19 +85,52 @@ describe('Generals', () => {
     expect(screen.queryByText('Select Network')).toBeTruthy();
   });
 
-  it('should render the custom derivation path field with no default value', () => {
-    jest.clearAllMocks();
-    accountFormInstance = renderWithStore(AddAccountForm, props, {
-      settings: { enableCustomDerivationPath: true },
+  it('should have disabled button if derivation path has an error', () => {
+    props.settings.enableCustomDerivationPath = true;
+    accountFormInstance.rerender(<AddAccountForm {...props} />);
+
+    const input = screen.getByLabelText('Custom derivation path');
+    fireEvent.change(input, { target: { value: 'incorrectPath' } });
+
+    const passphraseInput1 = screen.getByTestId('recovery-1');
+    const pasteEvent = createEvent.paste(passphraseInput1, {
+      clipboardData: {
+        getData: () =>
+          'below record evolve eye youth post control consider spice swamp hidden easily',
+      },
     });
+    fireEvent(passphraseInput1, pasteEvent);
 
-    expect(accountFormInstance.getByDisplayValue(defaultDerivationPath)).toBeTruthy();
+    expect(screen.getByText('Continue')).toHaveAttribute('disabled');
   });
 
-  it('should render the custom derivation path field with default value', () => {
-    accountFormInstance = renderWithStore(AddAccountForm, props, {
-      settings: { enableCustomDerivationPath: true, customDerivationPath: `m/0/2'` },
+  it('should trigger add account if derivation path and passphrase is correct', () => {
+    props.settings.enableCustomDerivationPath = true;
+    accountFormInstance.rerender(<AddAccountForm {...props} />);
+
+    const input = screen.getByLabelText('Custom derivation path');
+    const correctDerivationPath = "m/44'/134'/0'";
+    fireEvent.change(input, { target: { value: correctDerivationPath } });
+
+    const passphrase = 'below record evolve eye youth post control consider spice swamp hidden easily';
+    const passphraseInput1 = screen.getByTestId('recovery-1');
+    const pasteEvent = createEvent.paste(passphraseInput1, {
+      clipboardData: {
+        getData: () =>
+          passphrase,
+      },
     });
-    expect(screen.getByDisplayValue(`m/0/2'`)).toBeTruthy();
+    fireEvent(passphraseInput1, pasteEvent);
+
+    fireEvent.click(screen.getByText('Continue'));
+
+    expect(props.onAddAccount).toHaveBeenCalledWith({ value: passphrase, isValid: true }, correctDerivationPath);
+  });
+
+  it('should render the custom derivation path field with no default value', () => {
+    props.settings.enableCustomDerivationPath = true;
+    accountFormInstance.rerender(<AddAccountForm {...props} />);
+
+    expect(accountFormInstance.getByDisplayValue(defaultDerivationPath)).toBeTruthy();
   });
 });
diff --git a/src/modules/account/hooks/useEncryptAccount.js b/src/modules/account/hooks/useEncryptAccount.js
@@ -3,8 +3,8 @@ import { selectSettings } from 'src/redux/selectors';
 import { encryptAccount as encryptAccountUtils } from '@account/utils';
 
 // eslint-disable-next-line
-export function useEncryptAccount() {
-  const { enableCustomDerivationPath, customDerivationPath } = useSelector(selectSettings);
+export function useEncryptAccount(customDerivationPath) {
+  const { enableCustomDerivationPath } = useSelector(selectSettings);
   const encryptAccount = ({ recoveryPhrase, password, name }) => encryptAccountUtils({
     recoveryPhrase,
     password,

diff --git a/src/modules/auth/components/CustomDerivationPath/index.js b/src/modules/auth/components/CustomDerivationPath/index.js
@@ -1,32 +1,25 @@
 import React from 'react';
 import { useTranslation } from 'react-i18next';
-import { useDispatch, useSelector } from 'react-redux';
-import { settingsUpdated } from 'src/redux/actions';
-import { selectSettings } from 'src/redux/selectors';
 import { Input } from 'src/theme';
-import { defaultDerivationPath } from 'src/utils/explicitBipKeyDerivation';
-import styles from '../Signin/login.css';
 
-const CustomDerivationPath = () => {
-  const { enableCustomDerivationPath, customDerivationPath } = useSelector(selectSettings);
-  const dispatch = useDispatch();
+const CustomDerivationPath = ({ onChange, value, errorMessage }) => {
   const { t } = useTranslation();
 
   const onPathInputChange = (e) => {
-    dispatch(settingsUpdated({ customDerivationPath: e.target.value }));
+    onChange(e.target.value);
   };
 
-  if (!enableCustomDerivationPath) return null;
-
   return (
     <fieldset>
-      <label>{t('Custom derivation path')}</label>
+      <label htmlFor="custom-derivation-path-input">{t('Custom derivation path')}</label>
       <Input
-        className={`${styles.derivationPathInput} custom-derivation-path-input`}
-        size="l"
+        id="custom-derivation-path-input"
+        size="m"
         name="custom-derivation-path"
         onChange={onPathInputChange}
-        value={customDerivationPath || defaultDerivationPath}
+        value={value}
+        feedback={errorMessage}
+        error={!!errorMessage}
       />
     </fieldset>
   );

diff --git a/src/modules/auth/components/CustomDerivationPath/index.test.js b/src/modules/auth/components/CustomDerivationPath/index.test.js
@@ -0,0 +1,23 @@
+import { render, fireEvent } from '@testing-library/react';
+import React from 'react';
+
+import CustomDerivationPath from './index';
+
+describe('CustomDerivationPath', () => {
+  const props = {
+    onChange: jest.fn(),
+    value: '',
+  };
+
+  it('Should render without breaking', () => {
+    render(<CustomDerivationPath {...props} />);
+  });
+
+  it('Should call onChange when input changes', () => {
+    const { getByLabelText } = render(<CustomDerivationPath {...props} />);
+    const input = getByLabelText('Custom derivation path');
+    fireEvent.change(input, { target: { value: 'hello' } });
+
+    expect(props.onChange).toHaveBeenCalledWith('hello');
+  });
+});
diff --git a/src/modules/auth/components/SetPasswordForm/SetPasswordForm.js b/src/modules/auth/components/SetPasswordForm/SetPasswordForm.js
@@ -26,9 +26,9 @@ const setPasswordFormSchema = yup.object({
   hasAgreed: yup.boolean().required(),
 }).required();
 
-function SetPasswordForm({ onSubmit, recoveryPhrase }) {
+function SetPasswordForm({ onSubmit, recoveryPhrase, customDerivationPath }) {
   const { t } = useTranslation();
-  const { encryptAccount } = useEncryptAccount();
+  const { encryptAccount } = useEncryptAccount(customDerivationPath);
   const {
     register,
     handleSubmit,

diff --git a/src/modules/auth/components/Signin/login.css b/src/modules/auth/components/Signin/login.css
@@ -123,11 +123,6 @@
   }
 }
 
-.derivationPathInput {
-  height: 40px !important;
-  margin-top: var(--vertical-padding-m);
-}
-
 .link {
   @mixin contentLarge bold;
 

diff --git a/src/modules/auth/components/Signin/login.test.js b/src/modules/auth/components/Signin/login.test.js
@@ -1,11 +1,9 @@
 import React from 'react';
 import i18next from 'i18next';
 import { mount } from 'enzyme';
-import { useDispatch } from 'react-redux';
 import { mountWithRouterAndStore } from 'src/utils/testHelpers';
 import routes from 'src/routes/routes';
 import { defaultDerivationPath } from 'src/utils/explicitBipKeyDerivation';
-import { settingsUpdated } from 'src/redux/actions';
 import accounts from '@tests/constants/wallets';
 import Login from './login';
 
@@ -158,33 +156,5 @@ describe('Login', () => {
         wrapper.find('.custom-derivation-path-input').exists(),
       ).toBeFalsy();
     });
-
-    it('Should display custom derivation path and dispatch settings updated action', () => {
-      const mockDispatch = jest.fn();
-      useDispatch.mockReturnValue(mockDispatch);
-      wrapper = mountWithRouterAndStore(
-        Login,
-        props,
-        {},
-        {
-          settings: {
-            enableCustomDerivationPath: true,
-            customDerivationPath: defaultDerivationPath,
-          },
-        },
-      );
-
-      expect(
-        wrapper.find('.custom-derivation-path-input').at(1).props().value,
-      ).toBe(defaultDerivationPath);
-      wrapper
-        .find('.custom-derivation-path-input')
-        .at(1)
-        .simulate('change', { target: { value: "m/44'/134'/1'" } });
-      wrapper.update();
-      expect(mockDispatch).toHaveBeenCalledWith(
-        settingsUpdated({ customDerivationPath: "m/44'/134'/1'" }),
-      );
-    });
   });
 });
diff --git a/src/modules/wallet/components/PassphraseInput/PassphraseInput.js b/src/modules/wallet/components/PassphraseInput/PassphraseInput.js
@@ -17,7 +17,7 @@ class passphraseInput extends React.Component {
     this.state = {
       showPassphrase: false,
       partialPassphraseError: [],
-      values: [],
+      values: props.values || [],
       focus: 0,
       validationError: '',
       passphraseIsInvalid: false,