Revert "NetworkController refactor for new EthClient interface"

7 years ago · 8da0d0b28a
parent 5eeb144046
commit 8da0d0b28a
11 changed files with 96 additions and 105 deletions
--- a/app/scripts/controllers/network.js
+++ b/app/scripts/controllers/network.js
@ -1,12 +1,11 @@
 const assert = require('assert')
 const EventEmitter = require('events')
+const createMetamaskProvider = require('web3-provider-engine/zero.js')
 const ObservableStore = require('obs-store')
 const ComposedStore = require('obs-store/lib/composed')
 const extend = require('xtend')
 const EthQuery = require('eth-query')
-const createEthRpcClient = require('eth-rpc-client')
 const createEventEmitterProxy = require('../lib/events-proxy.js')
-const createObjectProxy = require('../lib/obj-proxy.js')
 const RPC_ADDRESS_LIST = require('../config.js').network
 const DEFAULT_RPC = RPC_ADDRESS_LIST['rinkeby']

@ -18,8 +17,7 @@ module.exports = class NetworkController extends EventEmitter {
    this.networkStore = new ObservableStore('loading')
    this.providerStore = new ObservableStore(config.provider)
    this.store = new ComposedStore({ provider: this.providerStore, network: this.networkStore })
-    this.providerProxy = createObjectProxy()
-    this.blockTrackerProxy = createEventEmitterProxy()
+    this._proxy = createEventEmitterProxy()

    this.on('networkDidChange', this.lookupNetwork)
  }
@ -27,11 +25,12 @@ module.exports = class NetworkController extends EventEmitter {
  initializeProvider (_providerParams) {
    this._baseProviderParams = _providerParams
    const rpcUrl = this.getCurrentRpcAddress()
-    this._configureStandardClient({ rpcUrl })
-    this.blockTrackerProxy.on('block', this._logBlock.bind(this))
-    this.blockTrackerProxy.on('error', this.verifyNetwork.bind(this))
-    this.ethQuery = new EthQuery(this.providerProxy)
+    this._configureStandardProvider({ rpcUrl })
+    this._proxy.on('block', this._logBlock.bind(this))
+    this._proxy.on('error', this.verifyNetwork.bind(this))
+    this.ethQuery = new EthQuery(this._proxy)
    this.lookupNetwork()
+    return this._proxy
  }

  verifyNetwork () {
@ -77,10 +76,8 @@ module.exports = class NetworkController extends EventEmitter {
    assert(type !== 'rpc', `NetworkController.setProviderType - cannot connect by type "rpc"`)
    // skip if type already matches
    if (type === this.getProviderConfig().type) return
-    // lookup rpcTarget for typecreateMetamaskProvider
    const rpcTarget = this.getRpcAddressForType(type)
    assert(rpcTarget, `NetworkController - unknown rpc address for type "${type}"`)
-    // update connectioncreateMetamaskProvider
    this.providerStore.updateState({ type, rpcTarget })
    this._switchNetwork({ rpcUrl: rpcTarget })
  }
@ -100,29 +97,32 @@ module.exports = class NetworkController extends EventEmitter {

  _switchNetwork (providerParams) {
    this.setNetworkState('loading')
-    this._configureStandardClient(providerParams)
+    this._configureStandardProvider(providerParams)
    this.emit('networkDidChange')
  }

-  _configureStandardClient(_providerParams) {
+  _configureStandardProvider(_providerParams) {
    const providerParams = extend(this._baseProviderParams, _providerParams)
-    const client = createEthRpcClient(providerParams)
-    this._setClient(client)
-  }
-
-  _setClient (newClient) {
-    // teardown old client
-    const oldClient = this._currentClient
-    if (oldClient) {
-      oldClient.blockTracker.stop()
-      // asyncEventEmitter lacks a "removeAllListeners" method
-      // oldClient.blockTracker.removeAllListeners
-      oldClient.blockTracker._events = {}
-    }
+    const provider = createMetamaskProvider(providerParams)
+    this._setProvider(provider)
+  }
+
+  _setProvider (provider) {
+    // collect old block tracker events
+    const oldProvider = this._provider
+    let blockTrackerHandlers
+    if (oldProvider) {
+      // capture old block handlers
+      blockTrackerHandlers = oldProvider._blockTracker.proxyEventHandlers
+      // tear down
+      oldProvider.removeAllListeners()
+      oldProvider.stop()
+    }
+    // override block tracler
+    provider._blockTracker = createEventEmitterProxy(provider._blockTracker, blockTrackerHandlers)
    // set as new provider
-    this._currentClient = newClient
-    this.providerProxy.setTarget(newClient.provider)
-    this.blockTrackerProxy.setTarget(newClient.blockTracker)
+    this._provider = provider
+    this._proxy.setTarget(provider)
  }

  _logBlock (block) {
--- a/app/scripts/controllers/transactions.js
+++ b/app/scripts/controllers/transactions.js
@ -46,7 +46,6 @@ module.exports = class TransactionController extends EventEmitter {
    this.txStateManager.on('tx:status-update', this.emit.bind(this, 'tx:status-update'))
    this.nonceTracker = new NonceTracker({
      provider: this.provider,
-      blockTracker: this.blockTracker,
      getPendingTransactions: this.txStateManager.getPendingTransactions.bind(this.txStateManager),
      getConfirmedTransactions: (address) => {
        return this.txStateManager.getFilteredTxList({
--- a/app/scripts/lib/events-proxy.js
+++ b/app/scripts/lib/events-proxy.js
@ -1,5 +1,6 @@
-module.exports = function createEventEmitterProxy(eventEmitter, eventHandlers = {}) {
+module.exports = function createEventEmitterProxy(eventEmitter, listeners) {
  let target = eventEmitter
+  const eventHandlers = listeners || {}
  const proxy = new Proxy({}, {
    get: (obj, name) => {
      // intercept listeners
@ -13,12 +14,9 @@ module.exports = function createEventEmitterProxy(eventEmitter, eventHandlers =
      return true
    },
  })
-  proxy.setTarget(eventEmitter)
-  return proxy
-
  function setTarget (eventEmitter) {
    target = eventEmitter
-    // migrate eventHandlers
+    // migrate listeners
    Object.keys(eventHandlers).forEach((name) => {
      eventHandlers[name].forEach((handler) => target.on(name, handler))
    })
@ -28,4 +26,6 @@ module.exports = function createEventEmitterProxy(eventEmitter, eventHandlers =
    eventHandlers[name].push(handler)
    target.on(name, handler)
  }
+  if (listeners) proxy.setTarget(eventEmitter)
+  return proxy
 }
--- a/app/scripts/lib/nonce-tracker.js
+++ b/app/scripts/lib/nonce-tracker.js
@ -4,9 +4,8 @@ const Mutex = require('await-semaphore').Mutex

 class NonceTracker {

-  constructor ({ provider, blockTracker, getPendingTransactions, getConfirmedTransactions }) {
+  constructor ({ provider, getPendingTransactions, getConfirmedTransactions }) {
    this.provider = provider
-    this.blockTracker = blockTracker
    this.ethQuery = new EthQuery(provider)
    this.getPendingTransactions = getPendingTransactions
    this.getConfirmedTransactions = getConfirmedTransactions
@ -54,7 +53,7 @@ class NonceTracker {
  }

  async _getCurrentBlock () {
-    const blockTracker = this.blockTracker
+    const blockTracker = this._getBlockTracker()
    const currentBlock = blockTracker.getCurrentBlock()
    if (currentBlock) return currentBlock
    return await Promise((reject, resolve) => {
@ -140,6 +139,11 @@ class NonceTracker {
    return { name: 'local', nonce: highest, details: { startPoint, highest } }
  }

+  // this is a hotfix for the fact that the blockTracker will
+  // change when the network changes
+  _getBlockTracker () {
+    return this.provider._blockTracker
+  }
 }

 module.exports = NonceTracker
--- a/app/scripts/lib/obj-proxy.js
+++ b/app/scripts/lib/obj-proxy.js
@ -1,19 +0,0 @@
-module.exports = function createObjectProxy(obj) {
-  let target = obj
-  const proxy = new Proxy({}, {
-    get: (obj, name) => {
-      // intercept setTarget
-      if (name === 'setTarget') return setTarget
-      return target[name]
-    },
-    set: (obj, name, value) => {
-      target[name] = value
-      return true
-    },
-  })
-  return proxy
-
-  function setTarget (obj) {
-    target = obj
-  }
-}
--- a/app/scripts/metamask-controller.js
+++ b/app/scripts/metamask-controller.js
@ -81,24 +81,9 @@ module.exports = class MetamaskController extends EventEmitter {
    })
    this.blacklistController.scheduleUpdates()

-    // rpc provider and block tracker
-    this.networkController.initializeProvider({
-      scaffold: {
-        eth_syncing: false,
-        web3_clientVersion: `MetaMask/v${version}`,
-      },
-      // account mgmt
-      getAccounts: nodeify(this.getAccounts, this),
-      // tx signing
-      processTransaction: nodeify(this.newTransaction, this),
-      // old style msg signing
-      processMessage: this.newUnsignedMessage.bind(this),
-      // personal_sign msg signing
-      processPersonalMessage: this.newUnsignedPersonalMessage.bind(this),
-      processTypedMessage: this.newUnsignedTypedMessage.bind(this),
-    })
-    this.provider = this.networkController.providerProxy
-    this.blockTracker = this.networkController.blockTrackerProxy
+    // rpc provider
+    this.provider = this.initializeProvider()
+    this.blockTracker = this.provider._blockTracker

    // eth data query tools
    this.ethQuery = new EthQuery(this.provider)
@ -233,6 +218,36 @@ module.exports = class MetamaskController extends EventEmitter {
  // Constructor helpers
  //

+  initializeProvider () {
+    const providerOpts = {
+      static: {
+        eth_syncing: false,
+        web3_clientVersion: `MetaMask/v${version}`,
+      },
+      // account mgmt
+      getAccounts: (cb) => {
+        const isUnlocked = this.keyringController.memStore.getState().isUnlocked
+        const result = []
+        const selectedAddress = this.preferencesController.getSelectedAddress()
+
+        // only show address if account is unlocked
+        if (isUnlocked && selectedAddress) {
+          result.push(selectedAddress)
+        }
+        cb(null, result)
+      },
+      // tx signing
+      processTransaction: nodeify(async (txParams) => await this.txController.newUnapprovedTransaction(txParams), this),
+      // old style msg signing
+      processMessage: this.newUnsignedMessage.bind(this),
+      // personal_sign msg signing
+      processPersonalMessage: this.newUnsignedPersonalMessage.bind(this),
+      processTypedMessage: this.newUnsignedTypedMessage.bind(this),
+    }
+    const providerProxy = this.networkController.initializeProvider(providerOpts)
+    return providerProxy
+  }
+
  initPublicConfigStore () {
    // get init state
    const publicConfigStore = new ObservableStore()
@ -468,18 +483,6 @@ module.exports = class MetamaskController extends EventEmitter {
  // Opinionated Keyring Management
  //

-  async getAccounts () {
-    const isUnlocked = this.keyringController.memStore.getState().isUnlocked
-    const result = []
-    const selectedAddress = this.preferencesController.getSelectedAddress()
-
-    // only show address if account is unlocked
-    if (isUnlocked && selectedAddress) {
-      result.push(selectedAddress)
-    }
-    return result
-  }
-
  addNewAccount (cb) {
    const primaryKeyring = this.keyringController.getKeyringsByType('HD Key Tree')[0]
    if (!primaryKeyring) return cb(new Error('MetamaskController - No HD Key Tree found'))
@ -526,11 +529,6 @@ module.exports = class MetamaskController extends EventEmitter {
  // Identity Management
  //

-  // this function wrappper lets us pass the fn reference before txController is instantiated
-  async newTransaction (txParams) {
-    return await this.txController.newUnapprovedTransaction(txParams)
-  }
-
  newUnsignedMessage (msgParams, cb) {
    const msgId = this.messageManager.addUnapprovedMessage(msgParams)
    this.sendUpdate()
--- a/package.json
+++ b/package.json
@ -71,11 +71,9 @@
    "eth-contract-metadata": "^1.1.4",
    "eth-hd-keyring": "^1.2.1",
    "eth-json-rpc-filters": "^1.2.2",
-    "eth-json-rpc-middleware": "^1.4.3",
    "eth-keyring-controller": "^2.1.0",
    "eth-phishing-detect": "^1.1.4",
    "eth-query": "^2.1.2",
-    "eth-rpc-client": "^1.1.3",
    "eth-sig-util": "^1.4.0",
    "eth-simple-keyring": "^1.2.0",
    "eth-token-tracker": "^1.1.4",
--- a/test/integration/lib/first-time.js
+++ b/test/integration/lib/first-time.js
@ -3,9 +3,6 @@ const PASSWORD = 'password123'
 QUnit.module('first time usage')

 QUnit.test('render init screen', (assert) => {
-  // intercept reload attempts
-  window.onbeforeunload = () => true
-
  const done = assert.async()
  runFirstTimeUsageTest(assert).then(done).catch((err) => {
    assert.notOk(err, `Error was thrown: ${err.stack}`)
--- a/test/unit/network-contoller-test.js
+++ b/test/unit/network-contoller-test.js
@ -14,15 +14,15 @@ describe('# Network Controller', function () {
      },
    })

-    networkController.initializeProvider(networkControllerProviderInit)
+    networkController.initializeProvider(networkControllerProviderInit, dummyProviderConstructor)
  })
  describe('network', function () {
    describe('#provider', function () {
      it('provider should be updatable without reassignment', function () {
-        networkController.initializeProvider(networkControllerProviderInit)
-        const providerProxy = networkController.providerProxy
-        providerProxy.setTarget({ test: true })
-        assert.ok(providerProxy.test)
+        networkController.initializeProvider(networkControllerProviderInit, dummyProviderConstructor)
+        const proxy = networkController._proxy
+        proxy.setTarget({ test: true, on: () => {} })
+        assert.ok(proxy.test)
      })
    })
    describe('#getNetworkState', function () {
@ -66,4 +66,19 @@ describe('# Network Controller', function () {
  })
 })

+function dummyProviderConstructor() {
+  return {
+    // provider
+    sendAsync: noop,
+    // block tracker
+    _blockTracker: {},
+    start: noop,
+    stop: noop,
+    on: noop,
+    addListener: noop,
+    once: noop,
+    removeAllListeners: noop,
+  }
+}
+
 function noop() {}
--- a/test/unit/nonce-tracker-test.js
+++ b/test/unit/nonce-tracker-test.js
@ -190,13 +190,12 @@ function generateNonceTrackerWith (pending, confirmed, providerStub = '0x0') {
  providerResultStub.result = providerStub
  const provider = {
    sendAsync: (_, cb) => { cb(undefined, providerResultStub) },
-  }
-  const blockTracker = {
+    _blockTracker: {
      getCurrentBlock: () => '0x11b568',
+    },
  }
  return new NonceTracker({
    provider,
-    blockTracker,
    getPendingTransactions,
    getConfirmedTransactions,
  })
--- a/ui/app/app.js
+++ b/ui/app/app.js
@ -319,7 +319,7 @@ App.prototype.renderNetworkDropdown = function () {
      [
        h('i.fa.fa-question-circle.fa-lg.menu-icon'),
        'Localhost 8545',
-        providerType === 'localhost' ? h('.check', '✓') : null,
+        activeNetwork === 'http://localhost:8545' ? h('.check', '✓') : null,
      ]
    ),