Move sigUtil and keyrings to external modules

These external modules now have their own test coverage and build enforcement. This allowed me to somewhat more easily add good tests around our personalSign strategy (held now in [eth-sig-util](https://github.com/flyswatter/eth-sig-util), and allow each of the keyrings to import that, etc.

10 files changed, 202 insertions, 261 deletions

diff --git a/app/scripts/keyring-controller.js b/app/scripts/keyring-controller.js
index b30161003..8c379b5b9 100644
--- a/app/scripts/keyring-controller.js
+++ b/app/scripts/keyring-controller.js
@@ -5,10 +5,10 @@ const EventEmitter = require('events').EventEmitter
 const ObservableStore = require('obs-store')
 const filter = require('promise-filter')
 const encryptor = require('browser-passworder')
-const normalizeAddress = require('./lib/sig-util').normalize
+const normalizeAddress = require('eth-sig-util').normalize
 // Keyrings:
-const SimpleKeyring = require('./keyrings/simple')
-const HdKeyring = require('./keyrings/hd')
+const SimpleKeyring = require('eth-simple-keyring')
+const HdKeyring = require('eth-hd-keyring')
 const keyringTypes = [
   SimpleKeyring,
   HdKeyring,
@@ -262,6 +262,35 @@ class KeyringController extends EventEmitter {
     })
   }
 
+  // Sign Personal Message
+  // @object msgParams
+  //
+  // returns Promise(@buffer rawSig)
+  //
+  // Attempts to sign the provided @object msgParams.
+  // Prefixes the hash before signing as per the new geth behavior.
+  signPersonalMessage (msgParams) {
+    const address = normalizeAddress(msgParams.from)
+    return this.getKeyringForAccount(address)
+    .then((keyring) => {
+      return keyring.signPersonalMessage(address, msgParams.data)
+    })
+  }
+
+  // Recover Personal Message
+  // @object msgParams
+  //
+  // returns Promise(@buffer signer)
+  //
+  // recovers a signature of the prefixed-style personalMessage signature.
+  recoverPersonalMessage (msgParams) {
+    const address = normalizeAddress(msgParams.from)
+    return this.getKeyringForAccount(address)
+    .then((keyring) => {
+      return keyring.recoverPersonalMessage(address, msgParams.data)
+    })
+  }
+
   // PRIVATE METHODS
   //
   // THESE METHODS ARE ONLY USED INTERNALLY TO THE KEYRING-CONTROLLER
diff --git a/app/scripts/keyrings/hd.js b/app/scripts/keyrings/hd.js
deleted file mode 100644
index 3a66f7868..000000000
--- a/app/scripts/keyrings/hd.js
+++ /dev/null
@@ -1,125 +0,0 @@
-const EventEmitter = require('events').EventEmitter
-const hdkey = require('ethereumjs-wallet/hdkey')
-const bip39 = require('bip39')
-const ethUtil = require('ethereumjs-util')
-
-// *Internal Deps
-const sigUtil = require('../lib/sig-util')
-
-// Options:
-const hdPathString = `m/44'/60'/0'/0`
-const type = 'HD Key Tree'
-
-class HdKeyring extends EventEmitter {
-
-  /* PUBLIC METHODS */
-
-  constructor (opts = {}) {
-    super()
-    this.type = type
-    this.deserialize(opts)
-  }
-
-  serialize () {
-    return Promise.resolve({
-      mnemonic: this.mnemonic,
-      numberOfAccounts: this.wallets.length,
-    })
-  }
-
-  deserialize (opts = {}) {
-    this.opts = opts || {}
-    this.wallets = []
-    this.mnemonic = null
-    this.root = null
-
-    if (opts.mnemonic) {
-      this._initFromMnemonic(opts.mnemonic)
-    }
-
-    if (opts.numberOfAccounts) {
-      return this.addAccounts(opts.numberOfAccounts)
-    }
-
-    return Promise.resolve([])
-  }
-
-  addAccounts (numberOfAccounts = 1) {
-    if (!this.root) {
-      this._initFromMnemonic(bip39.generateMnemonic())
-    }
-
-    const oldLen = this.wallets.length
-    const newWallets = []
-    for (let i = oldLen; i < numberOfAccounts + oldLen; i++) {
-      const child = this.root.deriveChild(i)
-      const wallet = child.getWallet()
-      newWallets.push(wallet)
-      this.wallets.push(wallet)
-    }
-    const hexWallets = newWallets.map(w => w.getAddress().toString('hex'))
-    return Promise.resolve(hexWallets)
-  }
-
-  getAccounts () {
-    return Promise.resolve(this.wallets.map(w => w.getAddress().toString('hex')))
-  }
-
-  // tx is an instance of the ethereumjs-transaction class.
-  signTransaction (address, tx) {
-    const wallet = this._getWalletForAccount(address)
-    var privKey = wallet.getPrivateKey()
-    tx.sign(privKey)
-    return Promise.resolve(tx)
-  }
-
-  // For eth_sign, we need to sign transactions:
-  // hd
-  signMessage (withAccount, data) {
-    const wallet = this._getWalletForAccount(withAccount)
-    const message = ethUtil.stripHexPrefix(data)
-    var privKey = wallet.getPrivateKey()
-    var msgSig = ethUtil.ecsign(new Buffer(message, 'hex'), privKey)
-    var rawMsgSig = ethUtil.bufferToHex(sigUtil.concatSig(msgSig.v, msgSig.r, msgSig.s))
-    return Promise.resolve(rawMsgSig)
-  }
-
-  // For eth_sign, we need to sign transactions:
-  newGethSignMessage (withAccount, msgHex) {
-    const wallet = this._getWalletForAccount(withAccount)
-    const privKey = wallet.getPrivateKey()
-    const msgBuffer = ethUtil.toBuffer(msgHex)
-    const msgHash = ethUtil.hashPersonalMessage(msgBuffer)
-    const msgSig = ethUtil.ecsign(msgHash, privKey)
-    const rawMsgSig = ethUtil.bufferToHex(sigUtil.concatSig(msgSig.v, msgSig.r, msgSig.s))
-    return Promise.resolve(rawMsgSig)
-  }
-
-  exportAccount (address) {
-    const wallet = this._getWalletForAccount(address)
-    return Promise.resolve(wallet.getPrivateKey().toString('hex'))
-  }
-
-
-  /* PRIVATE METHODS */
-
-  _initFromMnemonic (mnemonic) {
-    this.mnemonic = mnemonic
-    const seed = bip39.mnemonicToSeed(mnemonic)
-    this.hdWallet = hdkey.fromMasterSeed(seed)
-    this.root = this.hdWallet.derivePath(hdPathString)
-  }
-
-
-  _getWalletForAccount (account) {
-    const targetAddress = sigUtil.normalize(account)
-    return this.wallets.find((w) => {
-      const address = w.getAddress().toString('hex')
-      return ((address === targetAddress) ||
-              (sigUtil.normalize(address) === targetAddress))
-    })
-  }
-}
-
-HdKeyring.type = type
-module.exports = HdKeyring
diff --git a/app/scripts/keyrings/simple.js b/app/scripts/keyrings/simple.js
deleted file mode 100644
index 82881aa2d..000000000
--- a/app/scripts/keyrings/simple.js
+++ /dev/null
@@ -1,100 +0,0 @@
-const EventEmitter = require('events').EventEmitter
-const Wallet = require('ethereumjs-wallet')
-const ethUtil = require('ethereumjs-util')
-const type = 'Simple Key Pair'
-const sigUtil = require('../lib/sig-util')
-
-class SimpleKeyring extends EventEmitter {
-
-  /* PUBLIC METHODS */
-
-  constructor (opts) {
-    super()
-    this.type = type
-    this.opts = opts || {}
-    this.wallets = []
-  }
-
-  serialize () {
-    return Promise.resolve(this.wallets.map(w => w.getPrivateKey().toString('hex')))
-  }
-
-  deserialize (privateKeys = []) {
-    return new Promise((resolve, reject) => {
-      try {
-        this.wallets = privateKeys.map((privateKey) => {
-          const stripped = ethUtil.stripHexPrefix(privateKey)
-          const buffer = new Buffer(stripped, 'hex')
-          const wallet = Wallet.fromPrivateKey(buffer)
-          return wallet
-        })
-      } catch (e) {
-        reject(e)
-      }
-      resolve()
-    })
-  }
-
-  addAccounts (n = 1) {
-    var newWallets = []
-    for (var i = 0; i < n; i++) {
-      newWallets.push(Wallet.generate())
-    }
-    this.wallets = this.wallets.concat(newWallets)
-    const hexWallets = newWallets.map(w => ethUtil.bufferToHex(w.getAddress()))
-    return Promise.resolve(hexWallets)
-  }
-
-  getAccounts () {
-    return Promise.resolve(this.wallets.map(w => ethUtil.bufferToHex(w.getAddress())))
-  }
-
-  // tx is an instance of the ethereumjs-transaction class.
-  signTransaction (address, tx) {
-    const wallet = this._getWalletForAccount(address)
-    var privKey = wallet.getPrivateKey()
-    tx.sign(privKey)
-    return Promise.resolve(tx)
-  }
-
-  // For eth_sign, we need to sign transactions:
-  signMessage (withAccount, data) {
-    const wallet = this._getWalletForAccount(withAccount)
-    const message = ethUtil.stripHexPrefix(data)
-    var privKey = wallet.getPrivateKey()
-    var msgSig = ethUtil.ecsign(new Buffer(message, 'hex'), privKey)
-    var rawMsgSig = ethUtil.bufferToHex(sigUtil.concatSig(msgSig.v, msgSig.r, msgSig.s))
-    return Promise.resolve(rawMsgSig)
-  }
-
-  // For eth_sign, we need to sign transactions:
-
-  newGethSignMessage (withAccount, msgHex) {
-    const wallet = this._getWalletForAccount(withAccount)
-    const privKey = wallet.getPrivateKey()
-    const msgBuffer = ethUtil.toBuffer(msgHex)
-    const msgHash = ethUtil.hashPersonalMessage(msgBuffer)
-    const msgSig = ethUtil.ecsign(msgHash, privKey)
-    const rawMsgSig = ethUtil.bufferToHex(sigUtil.concatSig(msgSig.v, msgSig.r, msgSig.s))
-    return Promise.resolve(rawMsgSig)
-  }
-
-  exportAccount (address) {
-    const wallet = this._getWalletForAccount(address)
-    return Promise.resolve(wallet.getPrivateKey().toString('hex'))
-  }
-
-
-  /* PRIVATE METHODS */
-
-  _getWalletForAccount (account) {
-    const address = sigUtil.normalize(account)
-    let wallet = this.wallets.find(w => ethUtil.bufferToHex(w.getAddress()) === address)
-    if (!wallet) throw new Error('Simple Keyring - Unable to find matching address.')
-    return wallet
-  }
-
-}
-
-SimpleKeyring.type = type
-module.exports = SimpleKeyring
diff --git a/app/scripts/lib/config-manager.js b/app/scripts/lib/config-manager.js
index 6267eab68..ea5e49b19 100644
--- a/app/scripts/lib/config-manager.js
+++ b/app/scripts/lib/config-manager.js
@@ -1,6 +1,6 @@
 const MetamaskConfig = require('../config.js')
 const ethUtil = require('ethereumjs-util')
-const normalize = require('./sig-util').normalize
+const normalize = require('eth-sig-util').normalize
 
 const TESTNET_RPC = MetamaskConfig.network.testnet
 const MAINNET_RPC = MetamaskConfig.network.mainnet
diff --git a/app/scripts/lib/controllers/preferences.js b/app/scripts/lib/controllers/preferences.js
index dc9464c4e..c5e93a5b9 100644
--- a/app/scripts/lib/controllers/preferences.js
+++ b/app/scripts/lib/controllers/preferences.js
@@ -1,5 +1,5 @@
 const ObservableStore = require('obs-store')
-const normalizeAddress = require('../sig-util').normalize
+const normalizeAddress = require('eth-sig-util').normalize
 
 class PreferencesController {
 
diff --git a/app/scripts/lib/idStore-migrator.js b/app/scripts/lib/idStore-migrator.js
index 655aed0af..1485beb48 100644
--- a/app/scripts/lib/idStore-migrator.js
+++ b/app/scripts/lib/idStore-migrator.js
@@ -1,6 +1,6 @@
 const IdentityStore = require('./idStore')
 const HdKeyring = require('../keyrings/hd')
-const sigUtil = require('./sig-util')
+const sigUtil = require('eth-sig-util')
 const normalize = sigUtil.normalize
 const denodeify = require('denodeify')
 
diff --git a/app/scripts/lib/personal-message-manager.js b/app/scripts/lib/personal-message-manager.js
new file mode 100644
index 000000000..72dd1da96
--- /dev/null
+++ b/app/scripts/lib/personal-message-manager.js
@@ -0,0 +1,118 @@
+const EventEmitter = require('events')
+const ObservableStore = require('obs-store')
+const ethUtil = require('ethereumjs-util')
+const createId = require('./random-id')
+
+
+module.exports = class MessageManager extends EventEmitter{
+  constructor (opts) {
+    super()
+    this.memStore = new ObservableStore({
+      unapprovedPersonalMsgs: {},
+      unapprovedPersonalMsgCount: 0,
+    })
+    this.messages = []
+  }
+
+  get unapprovedPersonalMsgCount () {
+    return Object.keys(this.getUnapprovedMsgs()).length
+  }
+
+  getUnapprovedMsgs () {
+    return this.messages.filter(msg => msg.status === 'unapproved')
+    .reduce((result, msg) => { result[msg.id] = msg; return result }, {})
+  }
+
+  addUnapprovedMessage (msgParams) {
+    msgParams.data = normalizeMsgData(msgParams.data)
+    // create txData obj with parameters and meta data
+    var time = (new Date()).getTime()
+    var msgId = createId()
+    var msgData = {
+      id: msgId,
+      msgParams: msgParams,
+      time: time,
+      status: 'unapproved',
+    }
+    this.addMsg(msgData)
+
+    // signal update
+    this.emit('update')
+    return msgId
+  }
+
+  addMsg (msg) {
+    this.messages.push(msg)
+    this._saveMsgList()
+  }
+
+  getMsg (msgId) {
+    return this.messages.find(msg => msg.id === msgId)
+  }
+
+  approveMessage (msgParams) {
+    this.setMsgStatusApproved(msgParams.metamaskId)
+    return this.prepMsgForSigning(msgParams)
+  }
+
+  setMsgStatusApproved (msgId) {
+    this._setMsgStatus(msgId, 'approved')
+  }
+
+  setMsgStatusSigned (msgId, rawSig) {
+    const msg = this.getMsg(msgId)
+    msg.rawSig = rawSig
+    this._updateMsg(msg)
+    this._setMsgStatus(msgId, 'signed')
+  }
+
+  prepMsgForSigning (msgParams) {
+    delete msgParams.metamaskId
+    return Promise.resolve(msgParams)
+  }
+
+  rejectMsg (msgId) {
+    this._setMsgStatus(msgId, 'rejected')
+  }
+
+  //
+  // PRIVATE METHODS
+  //
+
+  _setMsgStatus (msgId, status) {
+    const msg = this.getMsg(msgId)
+    if (!msg) throw new Error('MessageManager - Message not found for id: "${msgId}".')
+    msg.status = status
+    this._updateMsg(msg)
+    this.emit(`${msgId}:${status}`, msg)
+    if (status === 'rejected' || status === 'signed') {
+      this.emit(`${msgId}:finished`, msg)
+    }
+  }
+
+  _updateMsg (msg) {
+    const index = this.messages.findIndex((message) => message.id === msg.id)
+    if (index !== -1) {
+      this.messages[index] = msg
+    }
+    this._saveMsgList()
+  }
+
+  _saveMsgList () {
+    const unapprovedPersonalMsgs = this.getUnapprovedMsgs()
+    const unapprovedPersonalMsgCount = Object.keys(unapprovedPersonalMsgs).length
+    this.memStore.updateState({ unapprovedPersonalMsgs, unapprovedPersonalMsgCount })
+    this.emit('updateBadge')
+  }
+
+}
+
+function normalizeMsgData(data) {
+  if (data.slice(0, 2) === '0x') {
+    // data is already hex
+    return data
+  } else {
+    // data is unicode, convert to hex
+    return ethUtil.bufferToHex(new Buffer(data, 'utf8'))
+  }
+}
diff --git a/app/scripts/lib/sig-util.js b/app/scripts/lib/sig-util.js
deleted file mode 100644
index 193dda381..000000000
--- a/app/scripts/lib/sig-util.js
+++ /dev/null
@@ -1,28 +0,0 @@
-const ethUtil = require('ethereumjs-util')
-
-module.exports = {
-
-  concatSig: function (v, r, s) {
-    const rSig = ethUtil.fromSigned(r)
-    const sSig = ethUtil.fromSigned(s)
-    const vSig = ethUtil.bufferToInt(v)
-    const rStr = padWithZeroes(ethUtil.toUnsigned(rSig).toString('hex'), 64)
-    const sStr = padWithZeroes(ethUtil.toUnsigned(sSig).toString('hex'), 64)
-    const vStr = ethUtil.stripHexPrefix(ethUtil.intToHex(vSig))
-    return ethUtil.addHexPrefix(rStr.concat(sStr, vStr)).toString('hex')
-  },
-
-  normalize: function (address) {
-    if (!address) return
-    return ethUtil.addHexPrefix(address.toLowerCase())
-  },
-
-}
-
-function padWithZeroes (number, length) {
-  var myString = '' + number
-  while (myString.length < length) {
-    myString = '0' + myString
-  }
-  return myString
-}
diff --git a/app/scripts/lib/tx-utils.js b/app/scripts/lib/tx-utils.js
index 5116cb93b..240a6ab47 100644
--- a/app/scripts/lib/tx-utils.js
+++ b/app/scripts/lib/tx-utils.js
@@ -2,7 +2,7 @@ const async = require('async')
 const EthQuery = require('eth-query')
 const ethUtil = require('ethereumjs-util')
 const Transaction = require('ethereumjs-tx')
-const normalize = require('./sig-util').normalize
+const normalize = require('eth-sig-util').normalize
 const BN = ethUtil.BN
 
 /*
diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js
index 29b13dc62..62242bd83 100644
--- a/app/scripts/metamask-controller.js
+++ b/app/scripts/metamask-controller.js
@@ -16,6 +16,7 @@ const CurrencyController = require('./lib/controllers/currency')
 const NoticeController = require('./notice-controller')
 const ShapeShiftController = require('./lib/controllers/shapeshift')
 const MessageManager = require('./lib/message-manager')
+const PersonalMessageManager = require('./lib/personal-message-manager')
 const TxManager = require('./transaction-manager')
 const ConfigManager = require('./lib/config-manager')
 const extension = require('./lib/extension')
@@ -23,6 +24,7 @@ const autoFaucet = require('./lib/auto-faucet')
 const nodeify = require('./lib/nodeify')
 const IdStoreMigrator = require('./lib/idStore-migrator')
 const accountImporter = require('./account-import-strategies')
+const sigUtil = require('eth-sig-util')
 
 const version = require('../manifest.json').version
 
@@ -105,6 +107,7 @@ module.exports = class MetamaskController extends EventEmitter {
 
     this.lookupNetwork()
     this.messageManager = new MessageManager()
+    this.personalMessageManager = new PersonalMessageManager()
     this.publicConfigStore = this.initPublicConfigStore()
 
     // TEMPORARY UNTIL FULL DEPRECATION:
@@ -163,8 +166,13 @@ module.exports = class MetamaskController extends EventEmitter {
       },
       // tx signing
       processTransaction: (txParams, cb) => this.newUnapprovedTransaction(txParams, cb),
-      // msg signing
+      // old style msg signing
       processMessage: this.newUnsignedMessage.bind(this),
+
+      // new style msg signing
+      approvePersonalMessage: this.approvePersonalMessage.bind(this),
+      signPersonalMessage:    this.signPersonalMessage.bind(this),
+      personalRecoverSigner:  this.personalRecoverSigner.bind(this),
     })
     return provider
   }
@@ -209,6 +217,7 @@ module.exports = class MetamaskController extends EventEmitter {
       this.ethStore.getState(),
       this.txManager.memStore.getState(),
       this.messageManager.memStore.getState(),
+      this.personalMessageManager.memStore.getState(),
       this.keyringController.memStore.getState(),
       this.preferencesController.store.getState(),
       this.currencyController.store.getState(),
@@ -449,6 +458,44 @@ module.exports = class MetamaskController extends EventEmitter {
     )(cb)
   }
 
+  // Prefixed Style Message Signing Methods:
+  approvePersonalMessage (cb) {
+    let msgId = this.personalMessageManager.addUnapprovedMessage(msgParams)
+    this.sendUpdate()
+    this.opts.showUnconfirmedMessage()
+    this.personalMessageManager.once(`${msgId}:finished`, (data) => {
+      switch (data.status) {
+        case 'signed':
+          return cb(null, data.rawSig)
+        case 'rejected':
+          return cb(new Error('MetaMask Message Signature: User denied transaction signature.'))
+        default:
+          return cb(new Error(`MetaMask Message Signature: Unknown problem: ${JSON.stringify(msgParams)}`))
+      }
+    })
+  }
+
+  signPersonalMessage (msgParams) {
+    const msgId = msgParams.metamaskId
+    // sets the status op the message to 'approved'
+    // and removes the metamaskId for signing
+    return this.personalMessageManager.approveMessage(msgParams)
+    .then((cleanMsgParams) => {
+      // signs the message
+      return this.keyringController.signPersonalMessage(cleanMsgParams)
+    })
+    .then((rawSig) => {
+      // tells the listener that the message has been signed
+      // and can be returned to the dapp
+      this.personalMessageManager.setMsgStatusSigned(msgId, rawSig)
+      return rawSig
+    })
+  }
+
+  personalRecoverSigner (msgParams) {
+    const recovered = sigUtil.recoverPersonalSignature(msgParams)
+    return Promise.resolve(recovered)
+  }
 
   markAccountsFound (cb) {
     this.configManager.setLostAccounts([])