Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix custom derivation path #4658

Merged
merged 19 commits into from
Dec 12, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
980606e
Fix alignment of warning icon when Input is of size m
oskarleonard Dec 7, 2022
e469afe
Remove custom derivation path from redux because it should not be per…
oskarleonard Dec 8, 2022
c43882a
Prevent the currently typed passphrase from being deleted when changi…
oskarleonard Dec 8, 2022
a8e8156
Remove test because customDerivationPath is not in redux store anymore
oskarleonard Dec 8, 2022
8ad4302
Use the defaultDerivationPath when enableCustomDerivationPath is true
oskarleonard Dec 8, 2022
195ab6e
Remove test for login component which we will deprecate
oskarleonard Dec 8, 2022
5b624eb
remove unused
oskarleonard Dec 8, 2022
7bbb2ed
Add tests for CustomDerivationPath
oskarleonard Dec 9, 2022
f323611
Use getByLabelText from render instead of importing it, also change t…
oskarleonard Dec 9, 2022
c485146
Add test to make sure that onAddAccount is called with both a passphr…
oskarleonard Dec 9, 2022
6a88ea3
Add test for when passphrase is correct but derivation path is incorr…
oskarleonard Dec 9, 2022
9dc7e99
Fix typo
oskarleonard Dec 9, 2022
7052f0a
Remove this second failsafe check since the button is already disable…
oskarleonard Dec 9, 2022
cf31cbf
Make sure disabled buttons dont have cursor pointer
oskarleonard Dec 9, 2022
3b3aafa
Merge branch 'development' into 4591-fix-custom-derivation-path
oskarleonard Dec 9, 2022
2761e8d
remove unused css rule
oskarleonard Dec 9, 2022
709ea87
Merge branch 'development' into 4591-fix-custom-derivation-path
oskarleonard Dec 9, 2022
49df8d4
Merge branch 'development' into 4591-fix-custom-derivation-path
oskarleonard Dec 12, 2022
af4cac2
add link to issue
oskarleonard Dec 12, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/locales/en/common.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
};

Expand All @@ -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}
Expand Down
64 changes: 58 additions & 6 deletions src/modules/account/components/AddAccountForm/AddAccountForm.js
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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) => {
Expand All @@ -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);
ManuGowda marked this conversation as resolved.
Show resolved Hide resolved
}
};

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']}`}>
Expand All @@ -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>
Expand All @@ -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;
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ beforeEach(() => {
});
});

describe('Generals', () => {
describe('AddAccountForm', () => {
it('should render successfully', () => {
expect(screen.getByText('Add account')).toBeTruthy();
expect(
Expand Down Expand Up @@ -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();
});
});
4 changes: 2 additions & 2 deletions src/modules/account/hooks/useEncryptAccount.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
23 changes: 8 additions & 15 deletions src/modules/auth/components/CustomDerivationPath/index.js
Original file line number Diff line number Diff line change
@@ -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>
);
Expand Down
23 changes: 23 additions & 0 deletions src/modules/auth/components/CustomDerivationPath/index.test.js
Original file line number Diff line number Diff line change
@@ -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');
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
5 changes: 0 additions & 5 deletions src/modules/auth/components/Signin/login.css
Original file line number Diff line number Diff line change
Expand Up @@ -123,11 +123,6 @@
}
}

.derivationPathInput {
height: 40px !important;
margin-top: var(--vertical-padding-m);
}

.link {
@mixin contentLarge bold;

Expand Down
30 changes: 0 additions & 30 deletions src/modules/auth/components/Signin/login.test.js
Original file line number Diff line number Diff line change
@@ -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';

Expand Down Expand Up @@ -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'" }),
);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ class passphraseInput extends React.Component {
this.state = {
showPassphrase: false,
partialPassphraseError: [],
values: [],
values: props.values || [],
focus: 0,
validationError: '',
passphraseIsInvalid: false,
Expand Down
Loading