code clan up and tests

3 files changed, 39 insertions, 41 deletions

diff --git a/app/scripts/keyring-controller.js b/app/scripts/keyring-controller.js
index 5fbd731f3..6b8260a28 100644
--- a/app/scripts/keyring-controller.js
+++ b/app/scripts/keyring-controller.js
@@ -6,7 +6,6 @@ const ObservableStore = require('obs-store')
 const filter = require('promise-filter')
 const encryptor = require('browser-passworder')
 const normalizeAddress = require('./lib/sig-util').normalize
-function noop () {}
 // Keyrings:
 const SimpleKeyring = require('./keyrings/simple')
 const HdKeyring = require('./keyrings/hd')
@@ -89,7 +88,6 @@ class KeyringController extends EventEmitter {
         currentFiat: this.configManager.getCurrentFiat(),
         conversionRate: this.configManager.getConversionRate(),
         conversionDate: this.configManager.getConversionDate(),
-        // messageManager
       }
     })
   }
@@ -319,7 +317,7 @@ class KeyringController extends EventEmitter {
   //
   // Attempts to sign the provided @object msgParams.
   signMessage (msgParams) {
-    const address = normalize(msgParams.from)
+    const address = normalizeAddress(msgParams.from)
     return this.getKeyringForAccount(address)
     .then((keyring) => {
       return keyring.signMessage(address, msgParams.data)
diff --git a/app/scripts/lib/message-manager.js b/app/scripts/lib/message-manager.js
index bc9a9e6c8..490cd4d1c 100644
--- a/app/scripts/lib/message-manager.js
+++ b/app/scripts/lib/message-manager.js
@@ -11,7 +11,7 @@ module.exports = class MessageManager extends EventEmitter{
 
   getState() {
     return {
-      unapprovedMsgs: this.unapprovedMsgs(),
+      unapprovedMsgs: this.getUnapprovedMsgs(),
       messages: this.getMsgList(),
     }
   }
@@ -21,22 +21,15 @@ module.exports = class MessageManager extends EventEmitter{
   }
 
   get unapprovedMsgCount () {
-    return Object.keys(this.unapprovedMsgs()).length
+    return Object.keys(this.getUnapprovedMsgs()).length
   }
 
-  unapprovedMsgs () {
+  getUnapprovedMsgs () {
     let messages = this.getMsgList()
     return messages.filter(msg => msg.status === 'unapproved')
     .reduce((result, msg) => { result[msg.id] = msg; return result }, {})
   }
 
-  _saveMsgList (msgList) {
-    this.emit('updateBadge')
-    let state = this.memStore.getState()
-    state.messages = msgList
-    this.memStore.putState(state)
-  }
-
   addUnapprovedMessage (msgParams) {
     // create txData obj with parameters and meta data
     var time = (new Date()).getTime()
@@ -70,33 +63,29 @@ module.exports = class MessageManager extends EventEmitter{
     return matching.length > 0 ? matching[0] : null
   }
 
-  brodcastMessage (rawSig, msgId, status) {
-    this.emit(`${msgId}:finished`, {status, rawSig})
-  }
-
   approveMessage (msgParams) {
-    this.setMessageApproved(msgParams.metamaskId)
+    this.setMsgStatusApproved(msgParams.metamaskId)
     return this.prepMsgForSigning(msgParams)
   }
 
-  setMessageApproved (msgId) {
+  setMsgStatusApproved (msgId) {
     this._setMsgStatus(msgId, 'approved')
   }
+
   prepMsgForSigning (msgParams) {
     delete msgParams.metamaskId
     return Promise.resolve(msgParams)
   }
 
-  cancelMessage (msgId) {
-    // reject tx
-    // clean up
+  rejectMsg (msgId) {
     this.brodcastMessage(null, msgId, 'rejected')
-    this.rejectMsg(msgId)
+    this._setMsgStatus(msgId, 'rejected')
   }
 
-  rejectMsg (msgId) {
-    this._setMsgStatus(msgId, 'rejected')
+  brodcastMessage (rawSig, msgId, status) {
+    this.emit(`${msgId}:finished`, {status, rawSig})
   }
+// PRIVATE METHODS
 
   _setMsgStatus (msgId, status) {
     let msg = this.getMsg(msgId)
@@ -112,4 +101,13 @@ module.exports = class MessageManager extends EventEmitter{
     }
     this._saveMsgList(messages)
   }
+
+  _saveMsgList (msgList) {
+    this.emit('updateBadge')
+    let state = this.memStore.getState()
+    state.messages = msgList
+    this.memStore.putState(state)
+  }
+
+
 }
diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js
index bab005af2..38358f04b 100644
--- a/app/scripts/metamask-controller.js
+++ b/app/scripts/metamask-controller.js
@@ -224,7 +224,7 @@ module.exports = class MetamaskController extends EventEmitter {
       approveTransaction:    txManager.approveTransaction.bind(txManager),
       cancelTransaction:     txManager.cancelTransaction.bind(txManager),
       signMessage: this.signMessage.bind(this),
-      cancelMessage: messageManager.cancelMessage.bind(messageManager),
+      cancelMessage: messageManager.rejectMsg.bind(messageManager),
 
       // notices
       checkNotices:   noticeController.updateNoticesList.bind(noticeController),
@@ -369,31 +369,33 @@ module.exports = class MetamaskController extends EventEmitter {
   }
 
   newUnsignedMessage (msgParams, cb) {
-    this.keyringController.getState()
-    .then((state) => {
-      let msgId = this.messageManager.addUnapprovedMessage(msgParams)
-      this.sendUpdate()
-      state.isUnlocked ? this.opts.unlockAccountMessage() : this.opts.showUnconfirmedMessage()
-      this.messageManager.once(`${msgId}:finished`, (data) => {
-        switch (data.status) {
-          case 'approved':
-            return cb(null, data.rawSig)
-          case 'rejected':
-            return cb(new Error('MetaMask Tx Signature: User denied transaction signature.'))
-          default:
-            return cb(new Error(`MetaMask Tx Signature: Unknown problem: ${JSON.stringify(msgParams)}`))
-        }
-      })
+    let msgId = this.messageManager.addUnapprovedMessage(msgParams)
+    this.sendUpdate()
+    this.opts.showUnconfirmedMessage()
+    this.messageManager.once(`${msgId}:finished`, (data) => {
+      switch (data.status) {
+        case 'approved':
+          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)}`))
+      }
     })
   }
 
   signMessage (msgParams, cb) {
     const msgId = msgParams.metamaskId
+    // sets the status op the message to 'approved'
+    // and removes the metamaskId for signing
     return this.messageManager.approveMessage(msgParams)
     .then((cleanMsgParams) => {
+      // signs the message
       return this.keyringController.signMessage(cleanMsgParams)
     })
     .then((rawSig) => {
+      // tells the listener that the message has been signed
+      // and can be returned to the dapp
       this.messageManager.brodcastMessage(rawSig, msgId, 'approved')
     }).then(() => {
       cb()