From a57d17a3e0f91ce44efe10bbc375d772d2d391b3 Mon Sep 17 00:00:00 2001 From: William O'Beirne Date: Mon, 30 Apr 2018 14:39:25 -0400 Subject: [PATCH] Fix Duplicate DPaths & Deterministic Modal Improvements (#1682) * Fix dPath uniqueness * Pass around dPath object, not just path, to maintain uniqueness * Improve deterministic modal UI * Fix custom dpaths --- .../components/DeterministicWalletsModal.scss | 14 ++++-- .../components/DeterministicWalletsModal.tsx | 50 +++++++++---------- .../WalletDecrypt/components/LedgerNano.tsx | 20 ++++---- .../WalletDecrypt/components/Mnemonic.tsx | 10 ++-- .../WalletDecrypt/components/Trezor.tsx | 22 ++++---- common/selectors/config/wallet.ts | 4 +- 6 files changed, 62 insertions(+), 58 deletions(-) diff --git a/common/components/WalletDecrypt/components/DeterministicWalletsModal.scss b/common/components/WalletDecrypt/components/DeterministicWalletsModal.scss index cbc474ee..88816a5f 100644 --- a/common/components/WalletDecrypt/components/DeterministicWalletsModal.scss +++ b/common/components/WalletDecrypt/components/DeterministicWalletsModal.scss @@ -46,7 +46,7 @@ } &-addresses { - overflow-y: scroll; + overflow-y: auto; &-table { width: 732px; text-align: center; @@ -74,13 +74,19 @@ background-image: url('~assets/images/icon-external-link.svg'); } + &-na { + font-size: $font-size-xs; + opacity: 0.3; + } + + // Specific selectors to override bootstrap tbody { tr { cursor: pointer; - } - td { - vertical-align: middle; + td { + vertical-align: middle; + } } } } diff --git a/common/components/WalletDecrypt/components/DeterministicWalletsModal.tsx b/common/components/WalletDecrypt/components/DeterministicWalletsModal.tsx index ee575938..80ba7135 100644 --- a/common/components/WalletDecrypt/components/DeterministicWalletsModal.tsx +++ b/common/components/WalletDecrypt/components/DeterministicWalletsModal.tsx @@ -24,7 +24,7 @@ const WALLETS_PER_PAGE = 5; interface Props { // Passed props isOpen?: boolean; - dPath: string; + dPath: DPath; dPaths: DPath[]; publicKey?: string; chainCode?: string; @@ -42,11 +42,11 @@ interface Props { onCancel(): void; onConfirmAddress(address: string, addressIndex: number): void; - onPathChange(path: string): void; + onPathChange(dPath: DPath): void; } interface State { - currentLabel: string; + currentDPath: DPath; selectedAddress: string; selectedAddrIndex: number; isCustomPath: boolean; @@ -65,7 +65,7 @@ class DeterministicWalletsModalClass extends React.PureComponent { selectedAddrIndex: 0, isCustomPath: false, customPath: '', - currentLabel: '', + currentDPath: this.props.dPath, page: 0 }; @@ -86,7 +86,7 @@ class DeterministicWalletsModalClass extends React.PureComponent { } public render() { - const { wallets, desiredToken, network, tokens, dPath, dPaths, onCancel } = this.props; + const { wallets, desiredToken, network, tokens, dPaths, onCancel } = this.props; const { selectedAddress, customPath, page } = this.state; const buttons: IButton[] = [ @@ -119,7 +119,7 @@ class DeterministicWalletsModalClass extends React.PureComponent {
{ const { dPath, publicKey, chainCode, seed } = props; if (dPath && ((publicKey && chainCode) || seed)) { - if (isValidPath(dPath)) { + if (isValidPath(dPath.value)) { this.props.getDeterministicWallets({ seed, - dPath, publicKey, chainCode, + dPath: dPath.value, limit: WALLETS_PER_PAGE, offset: WALLETS_PER_PAGE * this.state.page }); @@ -214,19 +214,12 @@ class DeterministicWalletsModalClass extends React.PureComponent { } } - private findDPath = (prop: keyof DPath, cmp: string) => { - return this.props.dPaths.find(d => d[prop] === cmp) || customDPath; - }; - private handleChangePath = (newPath: DPath) => { - const { value: dPathLabel } = newPath; - const { value } = this.findDPath('value', dPathLabel); - - if (value === customDPath.value) { - this.setState({ isCustomPath: true, currentLabel: dPathLabel }); + if (newPath.value === customDPath.value) { + this.setState({ isCustomPath: true, currentDPath: newPath }); } else { - this.setState({ isCustomPath: false, currentLabel: dPathLabel }); - this.props.onPathChange(value); + this.setState({ isCustomPath: false, currentDPath: newPath }); + this.props.onPathChange(newPath); } }; @@ -235,11 +228,14 @@ class DeterministicWalletsModalClass extends React.PureComponent { }; private handleSubmitCustomPath = (ev: React.FormEvent) => { - const { customPath, currentLabel } = this.state; + const { customPath, currentDPath } = this.state; ev.preventDefault(); - if (currentLabel === customDPath.label && isValidPath(customPath)) { - this.props.onPathChange(customPath); + if (currentDPath.value === customDPath.value && isValidPath(customPath)) { + this.props.onPathChange({ + label: customDPath.label, + value: customPath + }); } }; @@ -309,16 +305,16 @@ class DeterministicWalletsModalClass extends React.PureComponent { /> - {token ? ( + {desiredToken ? ( ) : ( - '???' + N/A )} diff --git a/common/components/WalletDecrypt/components/LedgerNano.tsx b/common/components/WalletDecrypt/components/LedgerNano.tsx index 06c36e0c..e4b3e222 100644 --- a/common/components/WalletDecrypt/components/LedgerNano.tsx +++ b/common/components/WalletDecrypt/components/LedgerNano.tsx @@ -26,7 +26,7 @@ interface StateProps { interface State { publicKey: string; chainCode: string; - dPath: string; + dPath: DPath; error: string | null; isLoading: boolean; showTip: boolean; @@ -38,7 +38,7 @@ class LedgerNanoSDecryptClass extends PureComponent { public state: State = { publicKey: '', chainCode: '', - dPath: this.props.dPath ? this.props.dPath.value : '', + dPath: this.props.dPath || this.props.dPaths[0], error: null, isLoading: false, showTip: false @@ -51,8 +51,8 @@ class LedgerNanoSDecryptClass extends PureComponent { }; public componentWillReceiveProps(nextProps: Props) { - if (this.props.dPath !== nextProps.dPath) { - this.setState({ dPath: nextProps.dPath ? nextProps.dPath.value : '' }); + if (this.props.dPath !== nextProps.dPath && nextProps.dPath) { + this.setState({ dPath: nextProps.dPath }); } } @@ -123,11 +123,11 @@ class LedgerNanoSDecryptClass extends PureComponent { ); } - private handlePathChange = (dPath: string) => { + private handlePathChange = (dPath: DPath) => { this.handleConnect(dPath); }; - private handleConnect = (dPath: string = this.state.dPath) => { + private handleConnect = (dPath: DPath) => { this.setState({ isLoading: true, error: null, @@ -136,7 +136,7 @@ class LedgerNanoSDecryptClass extends PureComponent { ledger.comm_u2f.create_async().then((comm: any) => { new ledger.eth(comm) - .getAddress_async(dPath, false, true) + .getAddress_async(dPath.value, false, true) .then(res => { this.setState({ publicKey: res.publicKey, @@ -182,19 +182,19 @@ class LedgerNanoSDecryptClass extends PureComponent { }; private handleUnlock = (address: string, index: number) => { - this.props.onUnlock(new LedgerWallet(address, this.state.dPath, index)); + this.props.onUnlock(new LedgerWallet(address, this.state.dPath.value, index)); this.reset(); }; private handleNullConnect = (): void => { - return this.handleConnect(); + return this.handleConnect(this.state.dPath); }; private reset() { this.setState({ publicKey: '', chainCode: '', - dPath: this.props.dPath ? this.props.dPath.value : '' + dPath: this.props.dPath || this.props.dPaths[0] }); } } diff --git a/common/components/WalletDecrypt/components/Mnemonic.tsx b/common/components/WalletDecrypt/components/Mnemonic.tsx index f255dbbb..0e3d30e1 100644 --- a/common/components/WalletDecrypt/components/Mnemonic.tsx +++ b/common/components/WalletDecrypt/components/Mnemonic.tsx @@ -27,7 +27,7 @@ interface State { formattedPhrase: string; pass: string; seed: string; - dPath: string; + dPath: DPath; } class MnemonicDecryptClass extends PureComponent { @@ -36,12 +36,12 @@ class MnemonicDecryptClass extends PureComponent { formattedPhrase: '', pass: '', seed: '', - dPath: this.props.dPath.value + dPath: this.props.dPath }; public componentWillReceiveProps(nextProps: Props) { if (this.props.dPath !== nextProps.dPath) { - this.setState({ dPath: nextProps.dPath.value }); + this.setState({ dPath: nextProps.dPath }); } } @@ -131,7 +131,7 @@ class MnemonicDecryptClass extends PureComponent { this.setState({ seed: '' }); }; - private handlePathChange = (dPath: string) => { + private handlePathChange = (dPath: DPath) => { this.setState({ dPath }); }; @@ -139,7 +139,7 @@ class MnemonicDecryptClass extends PureComponent { const { formattedPhrase, pass, dPath } = this.state; this.props.onUnlock({ - path: `${dPath}/${index}`, + path: `${dPath.value}/${index}`, pass, phrase: formattedPhrase, address diff --git a/common/components/WalletDecrypt/components/Trezor.tsx b/common/components/WalletDecrypt/components/Trezor.tsx index f2c09505..4b9e36f3 100644 --- a/common/components/WalletDecrypt/components/Trezor.tsx +++ b/common/components/WalletDecrypt/components/Trezor.tsx @@ -25,7 +25,7 @@ interface StateProps { interface State { publicKey: string; chainCode: string; - dPath: string; + dPath: DPath; error: string | null; isLoading: boolean; } @@ -36,14 +36,14 @@ class TrezorDecryptClass extends PureComponent { public state: State = { publicKey: '', chainCode: '', - dPath: this.props.dPath ? this.props.dPath.value : '', + dPath: this.props.dPath || this.props.dPaths[0], error: null, isLoading: false }; public componentWillReceiveProps(nextProps: Props) { - if (this.props.dPath !== nextProps.dPath) { - this.setState({ dPath: nextProps.dPath ? nextProps.dPath.value : '' }); + if (this.props.dPath !== nextProps.dPath && nextProps.dPath) { + this.setState({ dPath: nextProps.dPath }); } } @@ -98,19 +98,19 @@ class TrezorDecryptClass extends PureComponent { ); } - private handlePathChange = (dPath: string) => { + private handlePathChange = (dPath: DPath) => { this.setState({ dPath }); this.handleConnect(dPath); }; - private handleConnect = (dPath: string = this.state.dPath): void => { + private handleConnect = (dPath: DPath): void => { this.setState({ isLoading: true, error: null }); (TrezorConnect as any).getXPubKey( - dPath, + dPath.value, (res: any) => { if (res.success) { this.setState({ @@ -135,17 +135,19 @@ class TrezorDecryptClass extends PureComponent { }; private handleUnlock = (address: string, index: number) => { - this.props.onUnlock(new TrezorWallet(address, this.state.dPath, index)); + this.props.onUnlock(new TrezorWallet(address, this.state.dPath.value, index)); this.reset(); }; - private handleNullConnect = (): void => this.handleConnect(); + private handleNullConnect = (): void => { + this.handleConnect(this.state.dPath); + }; private reset() { this.setState({ publicKey: '', chainCode: '', - dPath: this.props.dPath ? this.props.dPath.value : '' + dPath: this.props.dPath || this.props.dPaths[0] }); } } diff --git a/common/selectors/config/wallet.ts b/common/selectors/config/wallet.ts index 02cddc08..f944dbdb 100644 --- a/common/selectors/config/wallet.ts +++ b/common/selectors/config/wallet.ts @@ -1,6 +1,6 @@ import { InsecureWalletName, SecureWalletName, WalletName, walletNames } from 'config'; import { EXTRA_PATHS } from 'config/dpaths'; -import sortedUniq from 'lodash/sortedUniq'; +import uniqBy from 'lodash/uniqBy'; import difference from 'lodash/difference'; import { StaticNetworkConfig, DPathFormats } from 'types/network'; import { AppState } from 'reducers'; @@ -22,7 +22,7 @@ export function getPaths(state: AppState, pathType: PathType): DPath[] { return networkPaths; }, []) .concat(EXTRA_PATHS); - return sortedUniq(paths); + return uniqBy(paths, p => `${p.label}${p.value}`); } export function getSingleDPath(state: AppState, format: DPathFormat): DPath | undefined {