code clan up and tests

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)

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,34 +63,30 @@ 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
-    this.brodcastMessage(null, msgId, 'rejected')
-    this.rejectMsg(msgId)
-  }
-
   rejectMsg (msgId) {
+    this.brodcastMessage(null, msgId, 'rejected')
     this._setMsgStatus(msgId, 'rejected')
   }
 
+  brodcastMessage (rawSig, msgId, status) {
+    this.emit(`${msgId}:finished`, {status, rawSig})
+  }
+// PRIVATE METHODS
+
   _setMsgStatus (msgId, status) {
     let msg = this.getMsg(msgId)
     if (msg) msg.status = status
@@ -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)
+  }
+
+
 }

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()

test/unit/message-manager-test.js

@@ -0,0 +1,98 @@
+const assert = require('assert')
+const extend = require('xtend')
+const EventEmitter = require('events')
+
+const MessageManger = require('../../app/scripts/lib/message-manager')
+
+describe('Transaction Manager', function() {
+  let messageManager
+
+  beforeEach(function() {
+    messageManager = new MessageManger ()
+  })
+
+  describe('#getMsgList', function() {
+    it('when new should return empty array', function() {
+      var result = messageManager.getMsgList()
+      assert.ok(Array.isArray(result))
+      assert.equal(result.length, 0)
+    })
+    it('should also return transactions from local storage if any', function() {
+
+    })
+  })
+
+  describe('#_saveMsgList', function() {
+    it('saves the submitted data to the Msg list', function() {
+      var target = [{ foo: 'bar', metamaskNetworkId: 'unit test' }]
+      messageManager._saveMsgList(target)
+      var result = messageManager.getMsgList()
+      assert.equal(result[0].foo, 'bar')
+    })
+  })
+
+  describe('#addMsg', function() {
+    it('adds a Msg returned in getMsgList', function() {
+      var Msg = { id: 1, status: 'approved', metamaskNetworkId: 'unit test' }
+      messageManager.addMsg(Msg)
+      var result = messageManager.getMsgList()
+      assert.ok(Array.isArray(result))
+      assert.equal(result.length, 1)
+      assert.equal(result[0].id, 1)
+    })
+  })
+
+  describe('#setMsgStatusApproved', function() {
+    it('sets the Msg status to approved', function() {
+      var Msg = { id: 1, status: 'unapproved', metamaskNetworkId: 'unit test' }
+      messageManager.addMsg(Msg)
+      messageManager.setMsgStatusApproved(1)
+      var result = messageManager.getMsgList()
+      assert.ok(Array.isArray(result))
+      assert.equal(result.length, 1)
+      assert.equal(result[0].status, 'approved')
+    })
+  })
+
+  describe('#rejectMsg', function() {
+    it('sets the Msg status to rejected', function() {
+      var Msg = { id: 1, status: 'unapproved', metamaskNetworkId: 'unit test' }
+      messageManager.addMsg(Msg)
+      messageManager.rejectMsg(1)
+      var result = messageManager.getMsgList()
+      assert.ok(Array.isArray(result))
+      assert.equal(result.length, 1)
+      assert.equal(result[0].status, 'rejected')
+    })
+  })
+
+  describe('#_updateMsg', function() {
+    it('replaces the Msg with the same id', function() {
+      messageManager.addMsg({ id: '1', status: 'unapproved', metamaskNetworkId: 'unit test' })
+      messageManager.addMsg({ id: '2', status: 'approved', metamaskNetworkId: 'unit test' })
+      messageManager._updateMsg({ id: '1', status: 'blah', hash: 'foo', metamaskNetworkId: 'unit test' })
+      var result = messageManager.getMsg('1')
+      assert.equal(result.hash, 'foo')
+    })
+  })
+
+  describe('#getUnapprovedMsgs', function() {
+    it('returns unapproved Msgs in a hash', function() {
+      messageManager.addMsg({ id: '1', status: 'unapproved', metamaskNetworkId: 'unit test' })
+      messageManager.addMsg({ id: '2', status: 'approved', metamaskNetworkId: 'unit test' })
+      let result = messageManager.getUnapprovedMsgs()
+      assert.equal(typeof result, 'object')
+      assert.equal(result['1'].status, 'unapproved')
+      assert.equal(result['2'], undefined)
+    })
+  })
+
+  describe('#getMsg', function() {
+    it('returns a Msg with the requested id', function() {
+      messageManager.addMsg({ id: '1', status: 'unapproved', metamaskNetworkId: 'unit test' })
+      messageManager.addMsg({ id: '2', status: 'approved', metamaskNetworkId: 'unit test' })
+      assert.equal(messageManager.getMsg('1').status, 'unapproved')
+      assert.equal(messageManager.getMsg('2').status, 'approved')
+    })
+  })
+})

ui/app/css/index.css

@@ -413,9 +413,9 @@ input.large-input {
   height: 24px;
   background: #4dffff;
   border: solid;
-  borderColor: #AEAEAE;
-  borderWidth: 0.5px;
-  borderRadius: 13px;
+  border-color: #AEAEAE;
+  border-width: 0.5px;
+  border-radius: 13px;
 }
 
 .edit-text {