Merge branch 'dev' into messageManagerCleanUp

4 files changed, 205 insertions, 194 deletions

diff --git a/app/scripts/background.js b/app/scripts/background.js
index 18882e5d5..da9c4f24b 100644
--- a/app/scripts/background.js
+++ b/app/scripts/background.js
@@ -1,6 +1,5 @@
 const urlUtil = require('url')
-const Dnode = require('dnode')
-const eos = require('end-of-stream')
+const endOfStream = require('end-of-stream')
 const asyncQ = require('async-q')
 const pipe = require('pump')
 const LocalStorageStore = require('obs-store/lib/localStorage')
@@ -10,7 +9,6 @@ const migrations = require('./migrations/')
 const PortStream = require('./lib/port-stream.js')
 const notification = require('./lib/notifications.js')
 const messageManager = require('./lib/message-manager')
-const setupMultiplex = require('./lib/stream-utils.js').setupMultiplex
 const MetamaskController = require('./metamask-controller')
 const extension = require('./lib/extension')
 const firstTimeState = require('./first-time-state')
@@ -93,52 +91,21 @@ function setupController (initState) {
     var portStream = new PortStream(remotePort)
     if (isMetaMaskInternalProcess) {
       // communication with popup
-      popupIsOpen = remotePort.name === 'popup'
-      setupTrustedCommunication(portStream, 'MetaMask', remotePort.name)
+      popupIsOpen = popupIsOpen || (remotePort.name === 'popup')
+      controller.setupTrustedCommunication(portStream, 'MetaMask', remotePort.name)
+      // record popup as closed
+      if (remotePort.name === 'popup') {
+        endOfStream(portStream, () => {
+          popupIsOpen = false
+        })
+      }
     } else {
       // communication with page
       var originDomain = urlUtil.parse(remotePort.sender.url).hostname
-      setupUntrustedCommunication(portStream, originDomain)
+      controller.setupUntrustedCommunication(portStream, originDomain)
     }
   }
 
-  function setupUntrustedCommunication (connectionStream, originDomain) {
-    // setup multiplexing
-    var mx = setupMultiplex(connectionStream)
-    // connect features
-    controller.setupProviderConnection(mx.createStream('provider'), originDomain)
-    controller.setupPublicConfig(mx.createStream('publicConfig'))
-  }
-
-  function setupTrustedCommunication (connectionStream, originDomain) {
-    // setup multiplexing
-    var mx = setupMultiplex(connectionStream)
-    // connect features
-    setupControllerConnection(mx.createStream('controller'))
-    controller.setupProviderConnection(mx.createStream('provider'), originDomain)
-  }
-
-  //
-  // remote features
-  //
-
-  function setupControllerConnection (stream) {
-    controller.stream = stream
-    var api = controller.getApi()
-    var dnode = Dnode(api)
-    stream.pipe(dnode).pipe(stream)
-    dnode.on('remote', (remote) => {
-      // push updates to popup
-      var sendUpdate = remote.sendUpdate.bind(remote)
-      controller.on('update', sendUpdate)
-      // teardown on disconnect
-      eos(stream, () => {
-        controller.removeListener('update', sendUpdate)
-        popupIsOpen = false
-      })
-    })
-  }
-
   //
   // User Interface setup
   //
diff --git a/app/scripts/keyring-controller.js b/app/scripts/keyring-controller.js
index 0a1965782..a4bee8ae1 100644
--- a/app/scripts/keyring-controller.js
+++ b/app/scripts/keyring-controller.js
@@ -154,17 +154,6 @@ module.exports = class KeyringController extends EventEmitter {
     .then(this.fullUpdate.bind(this))
   }
 
-  // ClearSeedWordCache
-  //
-  // returns Promise( @string currentSelectedAccount )
-  //
-  // Removes the current vault's seed words from the UI's state tree,
-  // ensuring they are only ever available in the background process.
-  clearSeedWordCache () {
-    this.configManager.setSeedWords(null)
-    return Promise.resolve(this.configManager.getSelectedAccount())
-  }
-
   // Set Locked
   // returns Promise( @object state )
   //
@@ -215,8 +204,8 @@ module.exports = class KeyringController extends EventEmitter {
       this.keyrings.push(keyring)
       return this.setupAccounts(accounts)
     })
-    .then(() => { return this.password })
-    .then(this.persistAllKeyrings.bind(this))
+    .then(() => this.persistAllKeyrings())
+    .then(() => this.fullUpdate())
     .then(() => {
       return keyring
     })
diff --git a/app/scripts/lib/stream-utils.js b/app/scripts/lib/stream-utils.js
index 1b7b89d14..ba79990cc 100644
--- a/app/scripts/lib/stream-utils.js
+++ b/app/scripts/lib/stream-utils.js
@@ -1,4 +1,5 @@
 const Through = require('through2')
+const endOfStream = require('end-of-stream')
 const ObjectMultiplex = require('./obj-multiplex')
 
 module.exports = {
@@ -24,11 +25,11 @@ function jsonStringifyStream () {
 function setupMultiplex (connectionStream) {
   var mx = ObjectMultiplex()
   connectionStream.pipe(mx).pipe(connectionStream)
-  mx.on('error', function (err) {
-    console.error(err)
+  endOfStream(mx, function (err) {
+    if (err) console.error(err)
   })
-  connectionStream.on('error', function (err) {
-    console.error(err)
+  endOfStream(connectionStream, function (err) {
+    if (err) console.error(err)
     mx.destroy()
   })
   return mx
diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js
index 3e6ce0a2e..3ce9c2373 100644
--- a/app/scripts/metamask-controller.js
+++ b/app/scripts/metamask-controller.js
@@ -2,11 +2,14 @@ const EventEmitter = require('events')
 const extend = require('xtend')
 const promiseToCallback = require('promise-to-callback')
 const pipe = require('pump')
+const Dnode = require('dnode')
 const ObservableStore = require('obs-store')
 const storeTransform = require('obs-store/lib/transform')
 const EthStore = require('./lib/eth-store')
 const EthQuery = require('eth-query')
+const streamIntoProvider = require('web3-stream-provider/handler')
 const MetaMaskProvider = require('web3-provider-engine/zero.js')
+const setupMultiplex = require('./lib/stream-utils.js').setupMultiplex
 const KeyringController = require('./keyring-controller')
 const NoticeController = require('./notice-controller')
 const messageManager = require('./lib/message-manager')
@@ -95,6 +98,63 @@ module.exports = class MetamaskController extends EventEmitter {
     this.txManager.on('update', this.sendUpdate.bind(this))
   }
 
+  //
+  // Constructor helpers
+  //
+
+  initializeProvider () {
+    let provider = MetaMaskProvider({
+      static: {
+        eth_syncing: false,
+        web3_clientVersion: `MetaMask/v${version}`,
+      },
+      rpcUrl: this.configManager.getCurrentRpcAddress(),
+      // account mgmt
+      getAccounts: (cb) => {
+        let selectedAccount = this.configManager.getSelectedAccount()
+        let result = selectedAccount ? [selectedAccount] : []
+        cb(null, result)
+      },
+      // tx signing
+      processTransaction: (txParams, cb) => this.newUnapprovedTransaction(txParams, cb),
+      // msg signing
+      approveMessage: this.newUnsignedMessage.bind(this),
+      signMessage: (...args) => {
+        this.keyringController.signMessage(...args)
+        this.sendUpdate()
+      },
+    })
+    return provider
+  }
+
+  initPublicConfigStore () {
+    // get init state
+    const publicConfigStore = new ObservableStore()
+
+    // sync publicConfigStore with transform
+    pipe(
+      this.store,
+      storeTransform(selectPublicState),
+      publicConfigStore
+    )
+
+    function selectPublicState(state) {
+      const result = { selectedAccount: undefined }
+      try {
+        result.selectedAccount = state.config.selectedAccount
+      } catch (_) {
+        // thats fine, im sure it will be there next time...
+      }
+      return result
+    }
+
+    return publicConfigStore
+  }
+
+  //
+  // State Management
+  //
+
   getState () {
     return this.keyringController.getState()
     .then((keyringControllerState) => {
@@ -111,116 +171,100 @@ module.exports = class MetamaskController extends EventEmitter {
     })
   }
 
+  //
+  // Remote Features
+  //
+
   getApi () {
     const keyringController = this.keyringController
     const txManager = this.txManager
     const noticeController = this.noticeController
 
     return {
-      getState: nodeify(this.getState.bind(this)),
-      setRpcTarget: this.setRpcTarget.bind(this),
-      setProviderType: this.setProviderType.bind(this),
-      useEtherscanProvider: this.useEtherscanProvider.bind(this),
-      agreeToDisclaimer: this.agreeToDisclaimer.bind(this),
-      resetDisclaimer: this.resetDisclaimer.bind(this),
-      setCurrentFiat: this.setCurrentFiat.bind(this),
-      setTOSHash: this.setTOSHash.bind(this),
-      checkTOSChange: this.checkTOSChange.bind(this),
-      setGasMultiplier: this.setGasMultiplier.bind(this),
-      markAccountsFound: this.markAccountsFound.bind(this),
-
-      // forward directly to keyringController
-      createNewVaultAndKeychain: nodeify(keyringController.createNewVaultAndKeychain).bind(keyringController),
-      createNewVaultAndRestore: nodeify(keyringController.createNewVaultAndRestore).bind(keyringController),
-      // Adds the current vault's seed words to the UI's state tree.
-      //
-      // Used when creating a first vault, to allow confirmation.
-      // Also used when revealing the seed words in the confirmation view.
-      placeSeedWords: (cb) => {
-        const primaryKeyring = keyringController.getKeyringsByType('HD Key Tree')[0]
-        if (!primaryKeyring) return cb(new Error('MetamaskController - No HD Key Tree found'))
-        primaryKeyring.serialize()
-        .then((serialized) => {
-          const seedWords = serialized.mnemonic
-          this.configManager.setSeedWords(seedWords)
-          promiseToCallback(this.keyringController.fullUpdate())(cb)
-        })
-      },
-      clearSeedWordCache: nodeify(keyringController.clearSeedWordCache).bind(keyringController),
-      setLocked: nodeify(keyringController.setLocked).bind(keyringController),
-      submitPassword: (password, cb) => {
-        this.migrateOldVaultIfAny(password)
-        .then(keyringController.submitPassword.bind(keyringController, password))
-        .then((newState) => { cb(null, newState) })
-        .catch((reason) => { cb(reason) })
-      },
-      addNewKeyring: (type, opts, cb) => {
-        keyringController.addNewKeyring(type, opts)
-        .then(() => keyringController.fullUpdate())
-        .then((newState) => { cb(null, newState) })
-        .catch((reason) => { cb(reason) })
-      },
-      addNewAccount: (cb) => {
-        const primaryKeyring = keyringController.getKeyringsByType('HD Key Tree')[0]
-        if (!primaryKeyring) return cb(new Error('MetamaskController - No HD Key Tree found'))
-        promiseToCallback(keyringController.addNewAccount(primaryKeyring))(cb)
-      },
-      importAccountWithStrategy: (strategy, args, cb) => {
-        accountImporter.importAccount(strategy, args)
-        .then((privateKey) => {
-          return keyringController.addNewKeyring('Simple Key Pair', [ privateKey ])
-        })
-        .then(keyring => keyring.getAccounts())
-        .then((accounts) => keyringController.setSelectedAccount(accounts[0]))
-        .then(() => { cb(null, keyringController.fullUpdate()) })
-        .catch((reason) => { cb(reason) })
-      },
-      setSelectedAccount: nodeify(keyringController.setSelectedAccount).bind(keyringController),
-      saveAccountLabel: nodeify(keyringController.saveAccountLabel).bind(keyringController),
-      exportAccount: nodeify(keyringController.exportAccount).bind(keyringController),
-
-      // signing methods
-      approveTransaction: txManager.approveTransaction.bind(txManager),
-      cancelTransaction: txManager.cancelTransaction.bind(txManager),
-      signMessage: keyringController.signMessage.bind(keyringController),
-      cancelMessage: keyringController.cancelMessage.bind(keyringController),
-
+      // etc
+      getState:              nodeify(this.getState.bind(this)),
+      setRpcTarget:          this.setRpcTarget.bind(this),
+      setProviderType:       this.setProviderType.bind(this),
+      useEtherscanProvider:  this.useEtherscanProvider.bind(this),
+      agreeToDisclaimer:     this.agreeToDisclaimer.bind(this),
+      resetDisclaimer:       this.resetDisclaimer.bind(this),
+      setCurrentFiat:        this.setCurrentFiat.bind(this),
+      setTOSHash:            this.setTOSHash.bind(this),
+      checkTOSChange:        this.checkTOSChange.bind(this),
+      setGasMultiplier:      this.setGasMultiplier.bind(this),
+      markAccountsFound:     this.markAccountsFound.bind(this),
       // coinbase
       buyEth: this.buyEth.bind(this),
       // shapeshift
       createShapeShiftTx: this.createShapeShiftTx.bind(this),
+      
+      // primary HD keyring management
+      addNewAccount:              this.addNewAccount.bind(this),
+      placeSeedWords:             this.placeSeedWords.bind(this),
+      clearSeedWordCache:         this.clearSeedWordCache.bind(this),
+      importAccountWithStrategy:  this.importAccountWithStrategy.bind(this),
+
+      // vault management
+      submitPassword: this.submitPassword.bind(this),
+
+      // KeyringController
+      setLocked:                 nodeify(keyringController.setLocked).bind(keyringController),
+      createNewVaultAndKeychain: nodeify(keyringController.createNewVaultAndKeychain).bind(keyringController),
+      createNewVaultAndRestore:  nodeify(keyringController.createNewVaultAndRestore).bind(keyringController),
+      addNewKeyring:             nodeify(keyringController.addNewKeyring).bind(keyringController),
+      setSelectedAccount:        nodeify(keyringController.setSelectedAccount).bind(keyringController),
+      saveAccountLabel:          nodeify(keyringController.saveAccountLabel).bind(keyringController),
+      exportAccount:             nodeify(keyringController.exportAccount).bind(keyringController),
+
+      // signing methods
+      approveTransaction:    txManager.approveTransaction.bind(txManager),
+      cancelTransaction:     txManager.cancelTransaction.bind(txManager),
+      signMessage:           keyringController.signMessage.bind(keyringController),
+      cancelMessage:         keyringController.cancelMessage.bind(keyringController),
+
       // notices
-      checkNotices: noticeController.updateNoticesList.bind(noticeController),
+      checkNotices:   noticeController.updateNoticesList.bind(noticeController),
       markNoticeRead: noticeController.markNoticeRead.bind(noticeController),
     }
   }
 
-  setupProviderConnection (stream, originDomain) {
-    stream.on('data', this.onRpcRequest.bind(this, stream, originDomain))
+  setupUntrustedCommunication (connectionStream, originDomain) {
+    // setup multiplexing
+    var mx = setupMultiplex(connectionStream)
+    // connect features
+    this.setupProviderConnection(mx.createStream('provider'), originDomain)
+    this.setupPublicConfig(mx.createStream('publicConfig'))
   }
 
-  onRpcRequest (stream, originDomain, request) {
-    // handle rpc request
-    this.provider.sendAsync(request, function onPayloadHandled (err, response) {
-      logger(err, request, response)
-      if (response) {
-        try {
-          stream.write(response)
-        } catch (err) {
-          logger(err)
-        }
-      }
+  setupTrustedCommunication (connectionStream, originDomain) {
+    // setup multiplexing
+    var mx = setupMultiplex(connectionStream)
+    // connect features
+    this.setupControllerConnection(mx.createStream('controller'))
+    this.setupProviderConnection(mx.createStream('provider'), originDomain)
+  }
+
+  setupControllerConnection (outStream) {
+    const api = this.getApi()
+    const dnode = Dnode(api)
+    outStream.pipe(dnode).pipe(outStream)
+    dnode.on('remote', (remote) => {
+      // push updates to popup
+      const sendUpdate = remote.sendUpdate.bind(remote)
+      this.on('update', sendUpdate)
     })
+  }
 
+  setupProviderConnection (outStream, originDomain) {
+    streamIntoProvider(outStream, this.provider, logger)
     function logger (err, request, response) {
       if (err) return console.error(err)
-      if (!request.isMetamaskInternal) {
-        if (global.METAMASK_DEBUG) {
-          console.log(`RPC (${originDomain}):`, request, '->', response)
-        }
-        if (response.error) {
-          console.error('Error in RPC response:\n', response.error)
-        }
+      if (response.error) {
+        console.error('Error in RPC response:\n', response.error)
+      }
+      if (request.isMetamaskInternal) return
+      if (global.METAMASK_DEBUG) {
+        console.log(`RPC (${originDomain}):`, request, '->', response)
       }
     }
   }
@@ -232,55 +276,67 @@ module.exports = class MetamaskController extends EventEmitter {
     })
   }
 
-  initializeProvider () {
-    let provider = MetaMaskProvider({
-      static: {
-        eth_syncing: false,
-        web3_clientVersion: `MetaMask/v${version}`,
-      },
-      rpcUrl: this.configManager.getCurrentRpcAddress(),
-      // account mgmt
-      getAccounts: (cb) => {
-        let selectedAccount = this.configManager.getSelectedAccount()
-        let result = selectedAccount ? [selectedAccount] : []
-        cb(null, result)
-      },
-      // tx signing
-      processTransaction: (txParams, cb) => this.newUnapprovedTransaction(txParams, cb),
-      // msg signing
-      approveMessage: this.newUnsignedMessage.bind(this),
-      signMessage: (...args) => {
-        this.keyringController.signMessage(...args)
-        this.sendUpdate()
-      },
-    })
-    return provider
+  //
+  // Vault Management
+  //
+
+  submitPassword (password, cb) {
+    this.migrateOldVaultIfAny(password)
+    .then(this.keyringController.submitPassword.bind(this.keyringController, password))
+    .then((newState) => { cb(null, newState) })
+    .catch((reason) => { cb(reason) })
   }
 
-  initPublicConfigStore () {
-    // get init state
-    const publicConfigStore = new ObservableStore()
+  //
+  // Opinionated Keyring Management
+  //
 
-    // sync publicConfigStore with transform
-    pipe(
-      this.store,
-      storeTransform(selectPublicState),
-      publicConfigStore
-    )
+  addNewAccount (cb) {
+    const primaryKeyring = this.keyringController.getKeyringsByType('HD Key Tree')[0]
+    if (!primaryKeyring) return cb(new Error('MetamaskController - No HD Key Tree found'))
+    promiseToCallback(this.keyringController.addNewAccount(primaryKeyring))(cb)
+  }
 
-    function selectPublicState(state) {
-      const result = { selectedAccount: undefined }
-      try {
-        result.selectedAccount = state.config.selectedAccount
-      } catch (_) {
-        // thats fine, im sure it will be there next time...
-      }
-      return result
-    }
+  // Adds the current vault's seed words to the UI's state tree.
+  //
+  // Used when creating a first vault, to allow confirmation.
+  // Also used when revealing the seed words in the confirmation view.
+  placeSeedWords (cb) {
+    const primaryKeyring = this.keyringController.getKeyringsByType('HD Key Tree')[0]
+    if (!primaryKeyring) return cb(new Error('MetamaskController - No HD Key Tree found'))
+    primaryKeyring.serialize()
+    .then((serialized) => {
+      const seedWords = serialized.mnemonic
+      this.configManager.setSeedWords(seedWords)
+      promiseToCallback(this.keyringController.fullUpdate())(cb)
+    })
+  }
 
-    return publicConfigStore
+  // ClearSeedWordCache
+  //
+  // Removes the primary account's seed words from the UI's state tree,
+  // ensuring they are only ever available in the background process.
+  clearSeedWordCache (cb) {
+    this.configManager.setSeedWords(null)
+    cb(null, this.configManager.getSelectedAccount())
+  }
+
+  importAccountWithStrategy (strategy, args, cb) {
+    accountImporter.importAccount(strategy, args)
+    .then((privateKey) => {
+      return this.keyringController.addNewKeyring('Simple Key Pair', [ privateKey ])
+    })
+    .then(keyring => keyring.getAccounts())
+    .then((accounts) => this.keyringController.setSelectedAccount(accounts[0]))
+    .then(() => { cb(null, this.keyringController.fullUpdate()) })
+    .catch((reason) => { cb(reason) })
   }
 
+
+  //
+  // Identity Management
+  //
+
   newUnapprovedTransaction (txParams, cb) {
     const self = this
     self.txManager.addUnapprovedTransaction(txParams, (err, txMeta) => {
@@ -321,9 +377,7 @@ module.exports = class MetamaskController extends EventEmitter {
   setupPublicConfig (outStream) {
     pipe(
       this.publicConfigStore,
-      outStream,
-      // cleanup on disconnect
-      () => this.publicConfigStore.unpipe(outStream)
+      outStream
     )
   }