Merge pull request #1057 from MetaMask/kumavis-refactor1

background - various refactor, cleanups, and fixes

4 files changed, 44 insertions, 62 deletions

diff --git a/app/scripts/keyring-controller.js b/app/scripts/keyring-controller.js
index 741757c5a..0a1965782 100644
--- a/app/scripts/keyring-controller.js
+++ b/app/scripts/keyring-controller.js
@@ -41,17 +41,6 @@ module.exports = class KeyringController extends EventEmitter {
     this.getNetwork = opts.getNetwork
   }
 
-  // Set Store
-  //
-  // Allows setting the ethStore after the constructor.
-  // This is currently required because of the initialization order
-  // of the ethStore and this class.
-  //
-  // Eventually would be nice to be able to add this in the constructor.
-  setStore (ethStore) {
-    this.ethStore = ethStore
-  }
-
   // Full Update
   // returns Promise( @object state )
   //
@@ -651,7 +640,7 @@ module.exports = class KeyringController extends EventEmitter {
   clearKeyrings () {
     let accounts
     try {
-      accounts = Object.keys(this.ethStore._currentState.accounts)
+      accounts = Object.keys(this.ethStore.getState())
     } catch (e) {
       accounts = []
     }
diff --git a/app/scripts/lib/config-manager.js b/app/scripts/lib/config-manager.js
index 6d088906c..fd4ac511a 100644
--- a/app/scripts/lib/config-manager.js
+++ b/app/scripts/lib/config-manager.js
@@ -266,7 +266,7 @@ ConfigManager.prototype.setCurrentFiat = function (currency) {
 
 ConfigManager.prototype.getCurrentFiat = function () {
   var data = this.getData()
-  return data.fiatCurrency
+  return data.fiatCurrency || 'USD'
 }
 
 ConfigManager.prototype.updateConversionRate = function () {
diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js
index 8f157a45e..3e6ce0a2e 100644
--- a/app/scripts/metamask-controller.js
+++ b/app/scripts/metamask-controller.js
@@ -5,12 +5,12 @@ const pipe = require('pump')
 const ObservableStore = require('obs-store')
 const storeTransform = require('obs-store/lib/transform')
 const EthStore = require('./lib/eth-store')
+const EthQuery = require('eth-query')
 const MetaMaskProvider = require('web3-provider-engine/zero.js')
 const KeyringController = require('./keyring-controller')
 const NoticeController = require('./notice-controller')
 const messageManager = require('./lib/message-manager')
 const TxManager = require('./transaction-manager')
-const Web3 = require('web3')
 const ConfigManager = require('./lib/config-manager')
 const extension = require('./lib/extension')
 const autoFaucet = require('./lib/auto-faucet')
@@ -29,30 +29,33 @@ module.exports = class MetamaskController extends EventEmitter {
 
     // observable state store
     this.store = new ObservableStore(opts.initState)
+    
     // config manager
     this.configManager = new ConfigManager({
       store: this.store,
     })
+    this.configManager.updateConversionRate()
+
+    // rpc provider
+    this.provider = this.initializeProvider(opts)
+    this.provider.on('block', this.processBlock.bind(this))
+    this.provider.on('error', this.getNetwork.bind(this))
+
+    // eth data query tools
+    this.ethQuery = new EthQuery(this.provider)
+    this.ethStore = new EthStore(this.provider)
+    
     // key mgmt
     this.keyringController = new KeyringController({
+      ethStore: this.ethStore,
       configManager: this.configManager,
       getNetwork: this.getStateNetwork.bind(this),
     })
     this.keyringController.on('newAccount', (account) => {
       autoFaucet(account)
     })
-    // notices
-    this.noticeController = new NoticeController({
-      configManager: this.configManager,
-    })
-    this.noticeController.updateNoticesList()
-    // to be uncommented when retrieving notices from a remote server.
-    // this.noticeController.startPolling()
-    this.provider = this.initializeProvider(opts)
-    this.ethStore = new EthStore(this.provider)
-    this.keyringController.setStore(this.ethStore)
-    this.getNetwork()
-    this.messageManager = messageManager
+
+    // tx mgmt
     this.txManager = new TxManager({
       txList: this.configManager.getTxList(),
       txHistoryLimit: 40,
@@ -64,11 +67,18 @@ module.exports = class MetamaskController extends EventEmitter {
       provider: this.provider,
       blockTracker: this.provider,
     })
-    this.publicConfigStore = this.initPublicConfigStore()
+    
+    // notices
+    this.noticeController = new NoticeController({
+      configManager: this.configManager,
+    })
+    this.noticeController.updateNoticesList()
+    // to be uncommented when retrieving notices from a remote server.
+    // this.noticeController.startPolling()
 
-    var currentFiat = this.configManager.getCurrentFiat() || 'USD'
-    this.configManager.setCurrentFiat(currentFiat)
-    this.configManager.updateConversionRate()
+    this.getNetwork()
+    this.messageManager = messageManager
+    this.publicConfigStore = this.initPublicConfigStore()
 
     this.checkTOSChange()
 
@@ -79,6 +89,7 @@ module.exports = class MetamaskController extends EventEmitter {
       configManager: this.configManager,
     })
 
+    // manual state subscriptions
     this.ethStore.on('update', this.sendUpdate.bind(this))
     this.keyringController.on('update', this.sendUpdate.bind(this))
     this.txManager.on('update', this.sendUpdate.bind(this))
@@ -221,10 +232,8 @@ module.exports = class MetamaskController extends EventEmitter {
     })
   }
 
-  initializeProvider (opts) {
-    const keyringController = this.keyringController
-
-    var providerOpts = {
+  initializeProvider () {
+    let provider = MetaMaskProvider({
       static: {
         eth_syncing: false,
         web3_clientVersion: `MetaMask/v${version}`,
@@ -232,8 +241,8 @@ module.exports = class MetamaskController extends EventEmitter {
       rpcUrl: this.configManager.getCurrentRpcAddress(),
       // account mgmt
       getAccounts: (cb) => {
-        var selectedAccount = this.configManager.getSelectedAccount()
-        var result = selectedAccount ? [selectedAccount] : []
+        let selectedAccount = this.configManager.getSelectedAccount()
+        let result = selectedAccount ? [selectedAccount] : []
         cb(null, result)
       },
       // tx signing
@@ -241,18 +250,10 @@ module.exports = class MetamaskController extends EventEmitter {
       // msg signing
       approveMessage: this.newUnsignedMessage.bind(this),
       signMessage: (...args) => {
-        keyringController.signMessage(...args)
+        this.keyringController.signMessage(...args)
         this.sendUpdate()
       },
-    }
-
-    var provider = MetaMaskProvider(providerOpts)
-    var web3 = new Web3(provider)
-    this.web3 = web3
-    keyringController.web3 = web3
-    provider.on('block', this.processBlock.bind(this))
-    provider.on('error', this.getNetwork.bind(this))
-
+    })
     return provider
   }
 
@@ -320,7 +321,9 @@ module.exports = class MetamaskController extends EventEmitter {
   setupPublicConfig (outStream) {
     pipe(
       this.publicConfigStore,
-      outStream
+      outStream,
+      // cleanup on disconnect
+      () => this.publicConfigStore.unpipe(outStream)
     )
   }
 
@@ -449,7 +452,7 @@ module.exports = class MetamaskController extends EventEmitter {
       this.sendUpdate()
     }
 
-    this.web3.version.getNetwork((err, network) => {
+    this.ethQuery.sendAsync({ method: 'net_version' }, (err, network) => {
       if (err) {
         this.state.network = 'loading'
         return this.sendUpdate()
diff --git a/test/unit/config-manager-test.js b/test/unit/config-manager-test.js
index fa3929599..acc73ebb4 100644
--- a/test/unit/config-manager-test.js
+++ b/test/unit/config-manager-test.js
@@ -16,23 +16,13 @@ describe('config-manager', function() {
 
   describe('currency conversions', function() {
 
-    describe('#getCurrentFiat', function() {
-      it('should return undefined if no previous key exists', function() {
-        var result = configManager.getCurrentFiat()
-        assert.ok(!result)
-      })
-    })
-
     describe('#setCurrentFiat', function() {
-      it('should make getCurrentFiat return true once set', function() {
-        assert.equal(configManager.getCurrentFiat(), undefined)
-        configManager.setCurrentFiat('USD')
-        var result = configManager.getCurrentFiat()
-        assert.equal(result, 'USD')
+      it('should return USD as default', function() {
+        assert.equal(configManager.getCurrentFiat(), 'USD')
       })
 
-      it('should work with other currencies as well', function() {
-        assert.equal(configManager.getCurrentFiat(), undefined)
+      it('should be able to set to other currency', function() {
+        assert.equal(configManager.getCurrentFiat(), 'USD')
         configManager.setCurrentFiat('JPY')
         var result = configManager.getCurrentFiat()
         assert.equal(result, 'JPY')