[DDW-172] Handle duplicate wallets in import process (#1985)

nikolaglumac · web-flow · commit 567112a11188 · 2020-04-21T16:55:53.000+02:00
* [DDW-172] Adds CHANGELOG entry

* [DDW-172] Use wallet indexes for wallet import logic

* [DDW-172] Finish logic implementation

* [DDW-172] Bump cardano-wallet version
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,6 +5,7 @@ Changelog
 
 ### Fixes
 
+- Handle duplicate wallets in import process ([PR 1985](https://github.com/input-output-hk/daedalus/pull/1985))
 - Treat wallets with 100% syncing progress as synced wallets ([PR 1984](https://github.com/input-output-hk/daedalus/pull/1984))
 - Fixed `cardano-node` / `jormungandr` and `cardano-wallet` info on the "Diagnostics" screen ([PR 1980](https://github.com/input-output-hk/daedalus/pull/1980))
 - Persist "Blank screen fix" / "--safe-mode" flag between Daedalus restarts ([PR 1979](https://github.com/input-output-hk/daedalus/pull/1979))
diff --git a/nix/sources.json b/nix/sources.json
@@ -41,10 +41,10 @@
         "homepage": null,
         "owner": "input-output-hk",
         "repo": "cardano-wallet",
-        "rev": "745aaad67004855a84c51e400c6fa1d10aedb910",
-        "sha256": "03y85ipp283bwn5jk1dg4cc3304dak2szs31qhys9facjyygdf81",
+        "rev": "b8ae9821f27eb24a119aa06532519cce2ab11cbc",
+        "sha256": "1j5wv2mjbpqbjl9r9647gxq7jsv14xghizslrhcahrvkv1f5p8m4",
         "type": "tarball",
-        "url": "https://github.com/input-output-hk/cardano-wallet/archive/745aaad67004855a84c51e400c6fa1d10aedb910.tar.gz",
+        "url": "https://github.com/input-output-hk/cardano-wallet/archive/b8ae9821f27eb24a119aa06532519cce2ab11cbc.tar.gz",
         "url_template": "https://github.com/<owner>/<repo>/archive/<rev>.tar.gz",
         "version": "v2020-04-07"
     },
diff --git a/source/renderer/app/actions/wallet-migration-actions.js b/source/renderer/app/actions/wallet-migration-actions.js
@@ -5,8 +5,8 @@ export default class WalletMigrationActions {
   startMigration: Action<any> = new Action();
   finishMigration: Action<any> = new Action();
   resetMigration: Action<any> = new Action();
-  toggleWalletImportSelection: Action<string> = new Action();
-  updateWalletName: Action<{ id: string, name: string }> = new Action();
+  toggleWalletImportSelection: Action<{ index: number }> = new Action();
+  updateWalletName: Action<{ index: number, name: string }> = new Action();
   nextStep: Action<any> = new Action();
   selectExportSourcePath: Action<any> = new Action();
 }
diff --git a/source/renderer/app/components/wallet/wallet-import/WalletSelectImportDialog.js b/source/renderer/app/components/wallet/wallet-import/WalletSelectImportDialog.js
@@ -152,7 +152,7 @@ export default class WalletSelectImportDialog extends Component<Props> {
       statusIcon = (
         <Checkbox
           onChange={() => {
-            onToggleWalletImportSelection(wallet.id);
+            onToggleWalletImportSelection({ index: wallet.index });
           }}
           checked={wallet.import.status === WalletImportStatuses.PENDING}
           disabled={disabled}
@@ -219,7 +219,10 @@ export default class WalletSelectImportDialog extends Component<Props> {
     const walletsWithoutNames = exportedWallets.filter(
       ({ hasName }: ExportedByronWallet) => !hasName
     );
-    let walletIndex = 0;
+
+    // We use previous wallet id to detect wallet duplicates
+    let previousWalletId = '';
+    let rowNumber = 1;
 
     return (
       <ReactModal
@@ -248,12 +251,15 @@ export default class WalletSelectImportDialog extends Component<Props> {
             </div>
             <div className={styles.walletsContainer}>
               {walletsWithNames.map(wallet => {
-                walletIndex++;
-                return (
-                  <div className={styles.walletsRow} key={wallet.id}>
-                    <div
-                      className={styles.walletsCounter}
-                    >{`${walletIndex}.`}</div>
+                const isDuplicate = previousWalletId === wallet.id;
+                const walletRow = (
+                  <div
+                    className={styles.walletsRow}
+                    key={`${wallet.id}-${wallet.index}`}
+                  >
+                    <div className={styles.walletsCounter}>
+                      {!isDuplicate && `${rowNumber}.`}
+                    </div>
                     <div className={styles.walletsInputField}>
                       <InlineEditingSmallInput
                         isActive={false}
@@ -272,7 +278,7 @@ export default class WalletSelectImportDialog extends Component<Props> {
                         )}
                         onSubmit={(name: string) =>
                           onWalletNameChange({
-                            id: wallet.id,
+                            index: wallet.index,
                             name,
                           })
                         }
@@ -288,6 +294,11 @@ export default class WalletSelectImportDialog extends Component<Props> {
                     </div>
                   </div>
                 );
+                if (!isDuplicate) {
+                  previousWalletId = wallet.id;
+                  rowNumber++;
+                }
+                return walletRow;
               })}
 
               {!!walletsWithoutNames.length && (
@@ -311,12 +322,15 @@ export default class WalletSelectImportDialog extends Component<Props> {
               )}
 
               {walletsWithoutNames.map(wallet => {
-                walletIndex++;
-                return (
-                  <div className={styles.walletsRow} key={wallet.id}>
-                    <div
-                      className={styles.walletsCounter}
-                    >{`${walletIndex}.`}</div>
+                const isDuplicate = previousWalletId === wallet.id;
+                const walletRow = (
+                  <div
+                    className={styles.walletsRow}
+                    key={`${wallet.id}-${wallet.index}`}
+                  >
+                    <div className={styles.walletsCounter}>
+                      {!isDuplicate && `${rowNumber}.`}
+                    </div>
                     <div className={styles.walletsInputField}>
                       <InlineEditingSmallInput
                         isActive={false}
@@ -335,7 +349,7 @@ export default class WalletSelectImportDialog extends Component<Props> {
                         )}
                         onSubmit={(name: string) =>
                           onWalletNameChange({
-                            id: wallet.id,
+                            index: wallet.index,
                             name,
                           })
                         }
@@ -350,6 +364,11 @@ export default class WalletSelectImportDialog extends Component<Props> {
                     </div>
                   </div>
                 );
+                if (!isDuplicate) {
+                  previousWalletId = wallet.id;
+                  rowNumber++;
+                }
+                return walletRow;
               })}
             </div>
             <div className={styles.action}>
diff --git a/source/renderer/app/components/wallet/wallet-import/WalletSelectImportDialog.scss b/source/renderer/app/components/wallet/wallet-import/WalletSelectImportDialog.scss
@@ -204,6 +204,7 @@
           font-size: 16px;
           letter-spacing: 0.5px;
           line-height: 1.38;
+          min-width: 35px;
           padding-right: 20px;
         }
 
diff --git a/source/renderer/app/containers/wallet/dialogs/WalletImportDialogContainer.js b/source/renderer/app/containers/wallet/dialogs/WalletImportDialogContainer.js
@@ -25,12 +25,14 @@ export default class WalletImportDialogContainer extends Component<Props> {
     this.props.actions.walletMigration.finishMigration.trigger();
   };
 
-  onWalletNameChange = (params: { id: string, name: string }) => {
+  onWalletNameChange = (params: { index: number, name: string }) => {
     this.props.actions.walletMigration.updateWalletName.trigger(params);
   };
 
-  onToggleWalletImportSelection = (id: string) => {
-    this.props.actions.walletMigration.toggleWalletImportSelection.trigger(id);
+  onToggleWalletImportSelection = (params: { index: number }) => {
+    this.props.actions.walletMigration.toggleWalletImportSelection.trigger(
+      params
+    );
   };
 
   onSelectExportSourcePath = () => {
diff --git a/source/renderer/app/stores/WalletMigrationStore.js b/source/renderer/app/stores/WalletMigrationStore.js
@@ -1,5 +1,6 @@
 // @flow
 import { action, computed, observable, runInAction } from 'mobx';
+import { orderBy } from 'lodash';
 import Store from './lib/Store';
 import Request from './lib/LocalizedRequest';
 import Wallet from '../domains/Wallet';
@@ -88,6 +89,15 @@ export default class WalletMigrationStore extends Store {
   getExportedWalletById = (id: string): ?ExportedByronWallet =>
     this.exportedWallets.find(w => w.id === id);
 
+  getExportedWalletDuplicatesById = (
+    id: string,
+    index: number
+  ): Array<ExportedByronWallet> =>
+    this.exportedWallets.filter(w => w.id === id && w.index !== index);
+
+  getExportedWalletByIndex = (index: number): ?ExportedByronWallet =>
+    this.exportedWallets.find(w => w.index === index);
+
   @action _selectExportSourcePath = async () => {
     const params = {
       defaultPath: global.legacyStateDir,
@@ -116,34 +126,52 @@ export default class WalletMigrationStore extends Store {
     }
   };
 
-  @action _toggleWalletImportSelection = (id: string) => {
-    const wallet = this.getExportedWalletById(id);
+  @action _toggleWalletImportSelection = ({ index }: { index: number }) => {
+    const wallet = this.getExportedWalletByIndex(index);
     if (wallet) {
       const { status } = wallet.import;
       const isPending = status === WalletImportStatuses.PENDING;
       this._updateWalletImportStatus(
-        id,
+        index,
         isPending
           ? WalletImportStatuses.UNSTARTED
           : WalletImportStatuses.PENDING
       );
+
+      const walletDuplicates = this.getExportedWalletDuplicatesById(
+        wallet.id,
+        index
+      );
+      if (walletDuplicates.length) {
+        walletDuplicates.forEach(w => {
+          if (w.import.status === WalletImportStatuses.PENDING) {
+            w.import.status = WalletImportStatuses.UNSTARTED;
+          }
+        });
+      }
     }
   };
 
   @action _updateWalletImportStatus = (
-    id: string,
+    index: number,
     status: WalletImportStatus,
     error?: LocalizableError
   ) => {
-    const wallet = this.getExportedWalletById(id);
+    const wallet = this.getExportedWalletByIndex(index);
     if (wallet) {
       wallet.import.status = status;
       wallet.import.error = error || null;
     }
   };
 
-  @action _updateWalletName = ({ id, name }: { id: string, name: string }) => {
-    const wallet = this.getExportedWalletById(id);
+  @action _updateWalletName = ({
+    index,
+    name,
+  }: {
+    index: number,
+    name: string,
+  }) => {
+    const wallet = this.getExportedWalletByIndex(index);
     if (wallet) {
       wallet.name = name;
     }
@@ -164,18 +192,28 @@ export default class WalletMigrationStore extends Store {
       locale: this.stores.profile.currentLocale,
     });
     runInAction('update exportedWallets and exportErrors', () => {
-      this.exportedWallets = wallets.map(wallet => {
-        const hasName = wallet.name !== null;
-        const importedWallet = this.stores.wallets.getWalletById(
-          `legacy_${wallet.id}`
-        );
-        const isImported = typeof importedWallet !== 'undefined';
-        if (isImported && importedWallet) wallet.name = importedWallet.name;
-        const status = isImported
-          ? WalletImportStatuses.EXISTS
-          : WalletImportStatuses.UNSTARTED;
-        return { ...wallet, hasName, import: { status, error: null } };
+      this.exportedWallets = orderBy(
+        wallets.map(wallet => {
+          const hasName = wallet.name !== null;
+          const importedWallet = this.stores.wallets.getWalletById(
+            `legacy_${wallet.id}`
+          );
+          const isImported = typeof importedWallet !== 'undefined';
+          if (isImported && importedWallet) wallet.name = importedWallet.name;
+          const status = isImported
+            ? WalletImportStatuses.EXISTS
+            : WalletImportStatuses.UNSTARTED;
+          return { ...wallet, hasName, import: { status, error: null } };
+        }),
+        ['hasName', 'name', 'id', 'is_passphrase_empty'],
+        ['desc', 'asc', 'asc', 'asc']
+      );
+
+      // Guard against duplicated wallet ids
+      this.exportedWallets.forEach((wallet, index) => {
+        wallet.index = index + 1;
       });
+
       this.exportErrors =
         errors || !this.exportedWalletsCount ? 'No wallets found' : '';
     });
@@ -227,8 +265,8 @@ export default class WalletMigrationStore extends Store {
     // Reset restore requests to clear previous errors
     this.restoreExportedWalletRequest.reset();
 
-    const { id } = exportedWallet;
-    this._updateWalletImportStatus(id, WalletImportStatuses.RUNNING);
+    const { id, index } = exportedWallet;
+    this._updateWalletImportStatus(index, WalletImportStatuses.RUNNING);
     try {
       const restoredWallet = await this.restoreExportedWalletRequest.execute(
         exportedWallet
@@ -237,13 +275,30 @@ export default class WalletMigrationStore extends Store {
         throw new Error('Restored wallet was not received correctly');
 
       runInAction('update restoredWallets', () => {
-        this._updateWalletImportStatus(id, WalletImportStatuses.COMPLETED);
+        this._updateWalletImportStatus(index, WalletImportStatuses.COMPLETED);
+
+        const walletDuplicates = this.getExportedWalletDuplicatesById(
+          id,
+          index
+        );
+        if (walletDuplicates.length) {
+          walletDuplicates.forEach(w => {
+            if (w.import.status !== WalletImportStatuses.COMPLETED) {
+              w.import.status = WalletImportStatuses.COMPLETED;
+            }
+          });
+        }
+
         this.restoredWallets.push(restoredWallet);
       });
     } catch (error) {
       runInAction('update restorationErrors', () => {
         const { name, is_passphrase_empty: hasPassword } = exportedWallet;
-        this._updateWalletImportStatus(id, WalletImportStatuses.ERRORED, error);
+        this._updateWalletImportStatus(
+          index,
+          WalletImportStatuses.ERRORED,
+          error
+        );
         this.restorationErrors.push({
           error,
           wallet: { id, name, hasPassword },
diff --git a/source/renderer/app/types/walletExportTypes.js b/source/renderer/app/types/walletExportTypes.js
@@ -41,4 +41,5 @@ export type ExportedByronWallet = {
     status: WalletImportStatus,
     error: ?LocalizableError,
   },
+  index: number,
 };

Original file line number	Diff line number	Diff line change
`@@ -204,6 +204,7 @@`
`204`	`204`	`font-size: 16px;`
`205`	`205`	`letter-spacing: 0.5px;`
`206`	`206`	`line-height: 1.38;`
	`207`	`+ min-width: 35px;`
`207`	`208`	`padding-right: 20px;`
`208`	`209`	`}`
`209`	`210`