Fix vault encrypting & unlocking bug

This is only a bug in dev, but was committed yesterday.

Sometimes the `encrypt` method was being passed values other than the password as the encryption key, leading to un-unlockable vaults.

To find this, and avoid it for all time hereafter, I added several more steps to our oft-neglected integration test suite, which now fully initializes a vault, locks it, and unlocks it again, to make sure all of those steps definitely work always.

6 files changed, 53 insertions, 13 deletions

diff --git a/app/scripts/keyring-controller.js b/app/scripts/keyring-controller.js
index ac9409dbb..9cfc3af97 100644
--- a/app/scripts/keyring-controller.js
+++ b/app/scripts/keyring-controller.js
@@ -167,7 +167,7 @@ module.exports = class KeyringController extends EventEmitter {
       this.configManager.setSelectedAccount(hexAccount)
       return this.setupAccounts(accounts)
     })
-    .then(this.persistAllKeyrings.bind(this))
+    .then(this.persistAllKeyrings.bind(this, password))
     .then(this.fullUpdate.bind(this))
   }
 
@@ -226,9 +226,11 @@ module.exports = class KeyringController extends EventEmitter {
     })
     .then((keyrings) => {
       this.keyrings = keyrings
-      return this.setupAccounts()
+      return this.fullUpdate()
+    })
+    .catch((reason) => {
+      return reason
     })
-    .then(this.fullUpdate.bind(this))
   }
 
   // Add New Keyring
@@ -250,6 +252,7 @@ module.exports = class KeyringController extends EventEmitter {
       this.keyrings.push(keyring)
       return this.setupAccounts(accounts)
     })
+    .then(() => { return this.password })
     .then(this.persistAllKeyrings.bind(this))
     .then(() => {
       return keyring
@@ -692,6 +695,9 @@ module.exports = class KeyringController extends EventEmitter {
   // Takes an account address and an iterator representing
   // the current number of named accounts.
   getBalanceAndNickname (account) {
+    if (!account) {
+      throw new Error('Problem loading account.')
+    }
     const address = normalize(account)
     this.ethStore.addAccount(address)
     return this.createNickname(address)
@@ -725,7 +731,9 @@ module.exports = class KeyringController extends EventEmitter {
   // encrypts that array with the provided `password`,
   // and persists that encrypted string to storage.
   persistAllKeyrings (password = this.password) {
-    this.password = password
+    if (typeof password === 'string') {
+      this.password = password
+    }
     return Promise.all(this.keyrings.map((keyring) => {
       return Promise.all([keyring.type, keyring.serialize()])
       .then((serializedKeyringArray) => {
diff --git a/app/scripts/lib/config-manager.js b/app/scripts/lib/config-manager.js
index 8e63265d2..59cc2b63c 100644
--- a/app/scripts/lib/config-manager.js
+++ b/app/scripts/lib/config-manager.js
@@ -3,6 +3,7 @@ const MetamaskConfig = require('../config.js')
 const migrations = require('./migrations')
 const rp = require('request-promise')
 const ethUtil = require('ethereumjs-util')
+const normalize = require('./sig-util').normalize
 
 const TESTNET_RPC = MetamaskConfig.network.testnet
 const MAINNET_RPC = MetamaskConfig.network.mainnet
@@ -273,13 +274,13 @@ ConfigManager.prototype.getWalletNicknames = function () {
 }
 
 ConfigManager.prototype.nicknameForWallet = function (account) {
-  const address = ethUtil.addHexPrefix(account.toLowerCase())
+  const address = normalize(account)
   const nicknames = this.getWalletNicknames()
   return nicknames[address]
 }
 
 ConfigManager.prototype.setNicknameForWallet = function (account, nickname) {
-  const address = ethUtil.addHexPrefix(account.toLowerCase())
+  const address = normalize(account)
   const nicknames = this.getWalletNicknames()
   nicknames[address] = nickname
   var data = this.getData()
diff --git a/test/integration/lib/encryptor-test.js b/test/integration/lib/encryptor-test.js
index d42608152..897d22740 100644
--- a/test/integration/lib/encryptor-test.js
+++ b/test/integration/lib/encryptor-test.js
@@ -1,5 +1,7 @@
 var encryptor = require('../../../app/scripts/lib/encryptor')
 
+QUnit.module('encryptor')
+
 QUnit.test('encryptor:serializeBufferForStorage', function (assert) {
   assert.expect(1)
   var buf = new Buffer(2)
@@ -65,3 +67,5 @@ QUnit.test('encryptor:encrypt & decrypt with wrong password', function(assert) {
     done()
   })
 })
+
+
diff --git a/test/integration/lib/first-time.js b/test/integration/lib/first-time.js
index d2fe31878..12c573db1 100644
--- a/test/integration/lib/first-time.js
+++ b/test/integration/lib/first-time.js
@@ -1,5 +1,7 @@
 const PASSWORD = 'password123'
 
+QUnit.module('first time usage')
+
 QUnit.test('agree to terms', function (assert) {
   var done = assert.async()
   let app
diff --git a/ui/app/actions.js b/ui/app/actions.js
index 0cc55136f..41be1004c 100644
--- a/ui/app/actions.js
+++ b/ui/app/actions.js
@@ -5,7 +5,11 @@ var actions = {
   goHome: goHome,
   // menu state
   getNetworkStatus: 'getNetworkStatus',
-
+  // transition state
+  TRANSITION_FORWARD: 'TRANSITION_FORWARD',
+  TRANSITION_BACKWARD: 'TRANSITION_BACKWARD',
+  transitionForward,
+  transitionBackward,
   // remote state
   UPDATE_METAMASK_STATE: 'UPDATE_METAMASK_STATE',
   updateMetamaskState: updateMetamaskState,
@@ -166,16 +170,25 @@ function tryUnlockMetamask (password) {
       if (err) {
         dispatch(actions.unlockFailed(err.message))
       } else {
-        let selectedAccount
-        try {
-          selectedAccount = newState.metamask.selectedAccount
-        } catch (e) {}
-        dispatch(actions.unlockMetamask(selectedAccount))
+        dispatch(actions.transitionForward())
+        dispatch(actions.updateMetamaskState(newState))
       }
     })
   }
 }
 
+function transitionForward() {
+  return {
+    type: this.TRANSITION_FORWARD,
+  }
+}
+
+function transitionBackward() {
+  return {
+    type: this.TRANSITION_BACKWARD,
+  }
+}
+
 function confirmSeedWords () {
   return (dispatch) => {
     dispatch(actions.showLoadingIndication())
diff --git a/ui/app/reducers/app.js b/ui/app/reducers/app.js
index 1f40e90b3..67a926948 100644
--- a/ui/app/reducers/app.js
+++ b/ui/app/reducers/app.js
@@ -43,7 +43,19 @@ function reduceApp (state, action) {
 
   switch (action.type) {
 
-  // intialize
+    // transition methods
+
+    case actions.TRANSITION_FORWARD:
+      return extend(appState, {
+        transForward: true,
+      })
+
+    case actions.TRANSITION_BACKWARD:
+      return extend(appState, {
+        transForward: false,
+      })
+
+    // intialize
 
     case actions.SHOW_CREATE_VAULT:
       return extend(appState, {