From 36510956518907a5127ba1061065394364d194cd Mon Sep 17 00:00:00 2001
From: Jaco Greeff <jacogr@gmail.com>
Date: Thu, 23 Feb 2017 15:04:58 +0100
Subject: [PATCH] Fix Geth account import (#4641)

* Fix Geth import - actually pass addresses through

* Work from addresses returned via RPC

* Display actual addresses imported (not selected)

* Update tests to cover bug fixed
---
 js/src/api/rpc/parity/parity.js               |  2 +-
 js/src/api/rpc/parity/parity.spec.js          | 15 ++++++++++
 .../AccountDetailsGeth/accountDetailsGeth.js  |  6 ++--
 .../CreateAccount/createAccount.test.js       |  2 +-
 js/src/modals/CreateAccount/store.js          | 30 ++++++++++++-------
 5 files changed, 39 insertions(+), 16 deletions(-)

diff --git a/js/src/api/rpc/parity/parity.js b/js/src/api/rpc/parity/parity.js
index 0f1bd492c..027d367d3 100644
--- a/js/src/api/rpc/parity/parity.js
+++ b/js/src/api/rpc/parity/parity.js
@@ -207,7 +207,7 @@ export default class Parity {
 
   importGethAccounts (accounts) {
     return this._transport
-      .execute('parity_importGethAccounts', inAddresses)
+      .execute('parity_importGethAccounts', inAddresses(accounts))
       .then(outAddresses);
   }
 
diff --git a/js/src/api/rpc/parity/parity.spec.js b/js/src/api/rpc/parity/parity.spec.js
index e25d6668c..d7eb41047 100644
--- a/js/src/api/rpc/parity/parity.spec.js
+++ b/js/src/api/rpc/parity/parity.spec.js
@@ -73,6 +73,21 @@ describe('api/rpc/parity', () => {
     });
   });
 
+  describe('importGethAccounts', () => {
+    const ACCOUNTS = ['0x63cf90d3f0410092fc0fca41846f596223979195'];
+    let scope;
+
+    beforeEach(() => {
+      scope = mockHttp([{ method: 'parity_importGethAccounts', reply: { result: ACCOUNTS } }]);
+    });
+
+    it('passes the addresses through', () => {
+      return instance.importGethAccounts(ACCOUNTS).then((result) => {
+        expect(scope.body['parity_importGethAccounts'].params).to.deep.equal([ACCOUNTS]);
+      });
+    });
+  });
+
   describe('minGasPrice', () => {
     it('returns the min gasprice, formatted', () => {
       mockHttp([{ method: 'parity_minGasPrice', reply: { result: '0x123456' } }]);
diff --git a/js/src/modals/CreateAccount/AccountDetailsGeth/accountDetailsGeth.js b/js/src/modals/CreateAccount/AccountDetailsGeth/accountDetailsGeth.js
index 3654a624a..588586c62 100644
--- a/js/src/modals/CreateAccount/AccountDetailsGeth/accountDetailsGeth.js
+++ b/js/src/modals/CreateAccount/AccountDetailsGeth/accountDetailsGeth.js
@@ -27,7 +27,7 @@ export default class AccountDetailsGeth extends Component {
   }
 
   render () {
-    const { gethAddresses } = this.props.store;
+    const { gethImported } = this.props.store;
 
     return (
       <div>
@@ -36,12 +36,12 @@ export default class AccountDetailsGeth extends Component {
             id='createAccount.accountDetailsGeth.imported'
             defaultMessage='You have imported {number} addresses from the Geth keystore:'
             values={ {
-              number: gethAddresses.length
+              number: gethImported.length
             } }
           />
         </div>
         <div className={ styles.address }>
-          { this.formatAddresses(gethAddresses) }
+          { this.formatAddresses(gethImported) }
         </div>
       </div>
     );
diff --git a/js/src/modals/CreateAccount/createAccount.test.js b/js/src/modals/CreateAccount/createAccount.test.js
index 5c59d41bf..d66729aec 100644
--- a/js/src/modals/CreateAccount/createAccount.test.js
+++ b/js/src/modals/CreateAccount/createAccount.test.js
@@ -35,7 +35,7 @@ function createApi () {
     },
     parity: {
       generateSecretPhrase: sinon.stub().resolves('some account phrase'),
-      importGethAccounts: sinon.stub().resolves(),
+      importGethAccounts: sinon.stub().resolves(GETH_ADDRESSES),
       listGethAccounts: sinon.stub().resolves(GETH_ADDRESSES),
       newAccountFromPhrase: sinon.stub().resolves(ADDRESS),
       newAccountFromSecret: sinon.stub().resolves(ADDRESS),
diff --git a/js/src/modals/CreateAccount/store.js b/js/src/modals/CreateAccount/store.js
index c7083349d..37e28ae4f 100644
--- a/js/src/modals/CreateAccount/store.js
+++ b/js/src/modals/CreateAccount/store.js
@@ -32,6 +32,7 @@ export default class Store {
   @observable description = '';
   @observable gethAccountsAvailable = [];
   @observable gethAddresses = [];
+  @observable gethImported = [];
   @observable isBusy = false;
   @observable isWindowsPhrase = false;
   @observable name = '';
@@ -124,6 +125,10 @@ export default class Store {
     this.gethAccountsAvailable = [].concat(gethAccountsAvailable);
   }
 
+  @action setGethImported = (gethImported) => {
+    this.gethImported = gethImported;
+  }
+
   @action setWindowsPhrase = (isWindowsPhrase = false) => {
     this.isWindowsPhrase = isWindowsPhrase;
   }
@@ -234,20 +239,23 @@ export default class Store {
   createAccountFromGeth = (timestamp = Date.now()) => {
     return this._api.parity
       .importGethAccounts(this.gethAddresses.peek())
-      .then(() => {
-        return Promise.all(this.gethAddresses.map((address) => {
-          return this._api.parity.setAccountName(address, 'Geth Import');
-        }));
-      })
-      .then(() => {
-        return Promise.all(this.gethAddresses.map((address) => {
-          return this._api.parity.setAccountMeta(address, {
-            timestamp
+      .then((gethImported) => {
+        console.log('createAccountFromGeth', gethImported);
+
+        this.setGethImported(gethImported);
+
+        return Promise
+          .all(gethImported.map((address) => {
+            return this._api.parity.setAccountName(address, 'Geth Import');
+          }))
+          .then(() => {
+            return Promise.all(gethImported.map((address) => {
+              return this._api.parity.setAccountMeta(address, { timestamp });
+            }));
           });
-        }));
       })
       .catch((error) => {
-        console.error('createAccount', error);
+        console.error('createAccountFromGeth', error);
         throw error;
       });
   }