From cc56d0d2f61da576acc72b1e7f63df4015469267 Mon Sep 17 00:00:00 2001 From: kumavis Date: Thu, 24 Aug 2017 15:44:40 -0700 Subject: [PATCH 01/19] inpage - use json-rpc-engine for inpage-provider --- app/scripts/lib/inpage-provider.js | 46 +++++++----------------------- app/scripts/metamask-controller.js | 42 ++++++++++++++++++++------- package.json | 1 + 3 files changed, 43 insertions(+), 46 deletions(-) diff --git a/app/scripts/lib/inpage-provider.js b/app/scripts/lib/inpage-provider.js index fd032a673..de6e8b811 100644 --- a/app/scripts/lib/inpage-provider.js +++ b/app/scripts/lib/inpage-provider.js @@ -1,8 +1,8 @@ const pipe = require('pump') -const StreamProvider = require('web3-stream-provider') +const RpcEngine = require('json-rpc-engine') +const createStreamMiddleware = require('json-rpc-middleware-stream') const LocalStorageStore = require('obs-store') const ObjectMultiplex = require('./obj-multiplex') -const createRandomId = require('./random-id') module.exports = MetamaskInpageProvider @@ -30,38 +30,20 @@ function MetamaskInpageProvider (connectionStream) { multiStream.ignoreStream('phishing') // connect to async provider - const asyncProvider = self.asyncProvider = new StreamProvider() + const streamMiddleware = createStreamMiddleware() pipe( - asyncProvider, + streamMiddleware.stream, multiStream.createStream('provider'), - asyncProvider, + streamMiddleware.stream, (err) => logStreamDisconnectWarning('MetaMask RpcProvider', err) ) // start and stop polling to unblock first block lock - self.idMap = {} - // handle sendAsync requests via asyncProvider - self.sendAsync = function (payload, cb) { - // rewrite request ids - var request = eachJsonMessage(payload, (message) => { - var newId = createRandomId() - self.idMap[newId] = message.id - message.id = newId - return message - }) - // forward to asyncProvider - asyncProvider.sendAsync(request, function (err, res) { - if (err) return cb(err) - // transform messages to original ids - eachJsonMessage(res, (message) => { - var oldId = self.idMap[message.id] - delete self.idMap[message.id] - message.id = oldId - return message - }) - cb(null, res) - }) - } + // handle sendAsync requests via dapp-side rpc engine + const engine = new RpcEngine() + engine.push(streamMiddleware) + + self.sendAsync = engine.handle.bind(engine) } MetamaskInpageProvider.prototype.send = function (payload) { @@ -121,14 +103,6 @@ MetamaskInpageProvider.prototype.isMetaMask = true // util -function eachJsonMessage (payload, transformFn) { - if (Array.isArray(payload)) { - return payload.map(transformFn) - } else { - return transformFn(payload) - } -} - function logStreamDisconnectWarning (remoteLabel, err) { let warningMsg = `MetamaskInpageProvider - lost connection to ${remoteLabel}` if (err) warningMsg += '\n' + err.stack diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index a007d6fc5..e4b1b5975 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -6,7 +6,8 @@ const Dnode = require('dnode') const ObservableStore = require('obs-store') const EthStore = require('./lib/eth-store') const EthQuery = require('eth-query') -const streamIntoProvider = require('web3-stream-provider/handler') +const RpcEngine = require('json-rpc-engine') +const createEngineStream = require('json-rpc-middleware-stream/engineStream') const setupMultiplex = require('./lib/stream-utils.js').setupMultiplex const KeyringController = require('./keyring-controller') const NetworkController = require('./controllers/network') @@ -375,19 +376,40 @@ module.exports = class MetamaskController extends EventEmitter { } setupProviderConnection (outStream, originDomain) { - streamIntoProvider(outStream, this.provider, onRequest, onResponse) + const engine = new RpcEngine() + engine.push(originMiddleware) + engine.push(loggerMiddleware) + engine.push(createProviderMiddleware({ provider: this.provider })) + + // setup connection + const providerStream = createEngineStream({ engine }) + outStream.pipe(providerStream).pipe(outStream) + // append dapp origin domain to request - function onRequest (request) { - request.origin = originDomain + function originMiddleware (req, res, next, end) { + req.origin = originDomain + next() } // log rpc activity - function onResponse (err, request, response) { - if (err) return console.error(err) - if (response.error) { - console.error('Error in RPC response:\n', response) + function loggerMiddleware (req, res, next, end) { + next((cb) => { + if (res.error) { + console.error('Error in RPC response:\n', res) + } + if (req.isMetamaskInternal) return + log.info(`RPC (${originDomain}):`, req, '->', res) + cb() + }) + } + // forward requests to provider + function createProviderMiddleware({ provider }) { + return (req, res, next, end) => { + provider.sendAsync(req, (err, _res) => { + if (err) return end(err) + res.result = _res.result + end() + }) } - if (request.isMetamaskInternal) return - log.info(`RPC (${originDomain}):`, request, '->', response) } } diff --git a/package.json b/package.json index 9a09e6305..a0d241341 100644 --- a/package.json +++ b/package.json @@ -93,6 +93,7 @@ "iframe-stream": "^3.0.0", "inject-css": "^0.1.1", "jazzicon": "^1.2.0", + "json-rpc-middleware-stream": "^1.0.0", "loglevel": "^1.4.1", "metamask-logo": "^2.1.2", "mississippi": "^1.2.0", From 440a42bbc38ed53b64dc017fd56bd3281355df33 Mon Sep 17 00:00:00 2001 From: kumavis Date: Thu, 7 Sep 2017 10:08:07 -0700 Subject: [PATCH 02/19] inpage - add idRemapMiddleware --- app/scripts/lib/inpage-provider.js | 4 +++- package.json | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/app/scripts/lib/inpage-provider.js b/app/scripts/lib/inpage-provider.js index de6e8b811..c095846e1 100644 --- a/app/scripts/lib/inpage-provider.js +++ b/app/scripts/lib/inpage-provider.js @@ -1,5 +1,6 @@ const pipe = require('pump') const RpcEngine = require('json-rpc-engine') +const createIdRemapMiddleware = require('json-rpc-engine/src/idRemapMiddleware') const createStreamMiddleware = require('json-rpc-middleware-stream') const LocalStorageStore = require('obs-store') const ObjectMultiplex = require('./obj-multiplex') @@ -27,7 +28,7 @@ function MetamaskInpageProvider (connectionStream) { ) // ignore phishing warning message (handled elsewhere) - multiStream.ignoreStream('phishing') + multiStream.ignoreStream('phishing') // connect to async provider const streamMiddleware = createStreamMiddleware() @@ -41,6 +42,7 @@ function MetamaskInpageProvider (connectionStream) { // handle sendAsync requests via dapp-side rpc engine const engine = new RpcEngine() + engine.push(createIdRemapMiddleware()) engine.push(streamMiddleware) self.sendAsync = engine.handle.bind(engine) diff --git a/package.json b/package.json index a0d241341..7712707c6 100644 --- a/package.json +++ b/package.json @@ -93,6 +93,7 @@ "iframe-stream": "^3.0.0", "inject-css": "^0.1.1", "jazzicon": "^1.2.0", + "json-rpc-engine": "^3.1.0", "json-rpc-middleware-stream": "^1.0.0", "loglevel": "^1.4.1", "metamask-logo": "^2.1.2", From 57e4805c621155cd86169064f4aaba34b73644c6 Mon Sep 17 00:00:00 2001 From: kumavis Date: Thu, 7 Sep 2017 21:17:49 -0700 Subject: [PATCH 03/19] streams - use pump and published obj-multiplex --- app/scripts/lib/obj-multiplex.js | 48 ------------------------------ app/scripts/lib/port-stream.js | 16 ++-------- app/scripts/lib/stream-utils.js | 23 +++++++------- app/scripts/metamask-controller.js | 34 ++++++++++++++++----- package.json | 4 ++- 5 files changed, 44 insertions(+), 81 deletions(-) delete mode 100644 app/scripts/lib/obj-multiplex.js diff --git a/app/scripts/lib/obj-multiplex.js b/app/scripts/lib/obj-multiplex.js deleted file mode 100644 index 0034febe0..000000000 --- a/app/scripts/lib/obj-multiplex.js +++ /dev/null @@ -1,48 +0,0 @@ -const through = require('through2') - -module.exports = ObjectMultiplex - -function ObjectMultiplex (opts) { - opts = opts || {} - // create multiplexer - const mx = through.obj(function (chunk, enc, cb) { - const name = chunk.name - const data = chunk.data - if (!name) { - console.warn(`ObjectMultiplex - Malformed chunk without name "${chunk}"`) - return cb() - } - const substream = mx.streams[name] - if (!substream) { - console.warn(`ObjectMultiplex - orphaned data for stream "${name}"`) - } else { - if (substream.push) substream.push(data) - } - return cb() - }) - mx.streams = {} - // create substreams - mx.createStream = function (name) { - const substream = mx.streams[name] = through.obj(function (chunk, enc, cb) { - mx.push({ - name: name, - data: chunk, - }) - return cb() - }) - mx.on('end', function () { - return substream.emit('end') - }) - if (opts.error) { - mx.on('error', function () { - return substream.emit('error') - }) - } - return substream - } - // ignore streams (dont display orphaned data warning) - mx.ignoreStream = function (name) { - mx.streams[name] = true - } - return mx -} diff --git a/app/scripts/lib/port-stream.js b/app/scripts/lib/port-stream.js index 607a9c9ed..648d88087 100644 --- a/app/scripts/lib/port-stream.js +++ b/app/scripts/lib/port-stream.js @@ -1,5 +1,6 @@ const Duplex = require('readable-stream').Duplex const inherits = require('util').inherits +const noop = function(){} module.exports = PortDuplexStream @@ -20,20 +21,14 @@ PortDuplexStream.prototype._onMessage = function (msg) { if (Buffer.isBuffer(msg)) { delete msg._isBuffer var data = new Buffer(msg) - // console.log('PortDuplexStream - saw message as buffer', data) this.push(data) } else { - // console.log('PortDuplexStream - saw message', msg) this.push(msg) } } PortDuplexStream.prototype._onDisconnect = function () { - try { - this.push(null) - } catch (err) { - this.emit('error', err) - } + this.destroy() } // stream plumbing @@ -45,19 +40,12 @@ PortDuplexStream.prototype._write = function (msg, encoding, cb) { if (Buffer.isBuffer(msg)) { var data = msg.toJSON() data._isBuffer = true - // console.log('PortDuplexStream - sent message as buffer', data) this._port.postMessage(data) } else { - // console.log('PortDuplexStream - sent message', msg) this._port.postMessage(msg) } } catch (err) { - // console.error(err) return cb(new Error('PortDuplexStream - disconnected')) } cb() } - -// util - -function noop () {} diff --git a/app/scripts/lib/stream-utils.js b/app/scripts/lib/stream-utils.js index ba79990cc..89e2a359e 100644 --- a/app/scripts/lib/stream-utils.js +++ b/app/scripts/lib/stream-utils.js @@ -1,6 +1,7 @@ const Through = require('through2') const endOfStream = require('end-of-stream') -const ObjectMultiplex = require('./obj-multiplex') +const ObjectMultiplex = require('obj-multiplex') +const pump = require('pump') module.exports = { jsonParseStream: jsonParseStream, @@ -23,14 +24,14 @@ function jsonStringifyStream () { } function setupMultiplex (connectionStream) { - var mx = ObjectMultiplex() - connectionStream.pipe(mx).pipe(connectionStream) - endOfStream(mx, function (err) { - if (err) console.error(err) - }) - endOfStream(connectionStream, function (err) { - if (err) console.error(err) - mx.destroy() - }) - return mx + const mux = new ObjectMultiplex() + pump( + connectionStream, + mux, + connectionStream, + (err) => { + if (err) console.error(err) + } + ) + return mux } diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index e4b1b5975..1a6732338 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -1,7 +1,7 @@ const EventEmitter = require('events') const extend = require('xtend') const promiseToCallback = require('promise-to-callback') -const pipe = require('pump') +const pump = require('pump') const Dnode = require('dnode') const ObservableStore = require('obs-store') const EthStore = require('./lib/eth-store') @@ -367,7 +367,14 @@ module.exports = class MetamaskController extends EventEmitter { setupControllerConnection (outStream) { const api = this.getApi() const dnode = Dnode(api) - outStream.pipe(dnode).pipe(outStream) + pump( + outStream, + dnode, + outStream, + (err) => { + if (err) console.error(err) + } + ) dnode.on('remote', (remote) => { // push updates to popup const sendUpdate = remote.sendUpdate.bind(remote) @@ -376,20 +383,29 @@ module.exports = class MetamaskController extends EventEmitter { } setupProviderConnection (outStream, originDomain) { + // setup json rpc engine stack const engine = new RpcEngine() engine.push(originMiddleware) engine.push(loggerMiddleware) engine.push(createProviderMiddleware({ provider: this.provider })) - + // setup connection const providerStream = createEngineStream({ engine }) - outStream.pipe(providerStream).pipe(outStream) - + pump( + outStream, + providerStream, + outStream, + (err) => { + if (err) console.error(err) + } + ) + // append dapp origin domain to request function originMiddleware (req, res, next, end) { req.origin = originDomain next() } + // log rpc activity function loggerMiddleware (req, res, next, end) { next((cb) => { @@ -401,6 +417,7 @@ module.exports = class MetamaskController extends EventEmitter { cb() }) } + // forward requests to provider function createProviderMiddleware({ provider }) { return (req, res, next, end) => { @@ -414,9 +431,12 @@ module.exports = class MetamaskController extends EventEmitter { } setupPublicConfig (outStream) { - pipe( + pump( this.publicConfigStore, - outStream + outStream, + (err) => { + if (err) console.error(err) + } ) } diff --git a/package.json b/package.json index 7712707c6..295d33409 100644 --- a/package.json +++ b/package.json @@ -69,6 +69,7 @@ "eth-bin-to-ops": "^1.0.1", "eth-contract-metadata": "^1.1.4", "eth-hd-keyring": "^1.1.1", + "eth-json-rpc-filters": "^1.0.1", "eth-phishing-detect": "^1.1.4", "eth-query": "^2.1.2", "eth-sig-util": "^1.2.2", @@ -101,6 +102,7 @@ "mkdirp": "^0.5.1", "multiplex": "^6.7.0", "number-to-bn": "^1.7.0", + "obj-multiplex": "^1.0.0", "obs-store": "^2.3.1", "once": "^1.3.3", "ping-pong-stream": "^1.0.0", @@ -121,7 +123,7 @@ "react-select": "^1.0.0-rc.2", "react-simple-file-input": "^1.0.0", "react-tooltip-component": "^0.3.0", - "readable-stream": "^2.1.2", + "readable-stream": "^2.3.3", "redux": "^3.0.5", "redux-logger": "^3.0.6", "redux-thunk": "^2.2.0", From 0e8e655fdb9b65df151f23ede74807025226ab66 Mon Sep 17 00:00:00 2001 From: kumavis Date: Thu, 7 Sep 2017 21:19:24 -0700 Subject: [PATCH 04/19] inpage - distinguish pump vs pipe --- app/scripts/lib/inpage-provider.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/scripts/lib/inpage-provider.js b/app/scripts/lib/inpage-provider.js index c095846e1..eb24dfcab 100644 --- a/app/scripts/lib/inpage-provider.js +++ b/app/scripts/lib/inpage-provider.js @@ -1,4 +1,4 @@ -const pipe = require('pump') +const pump = require('pump') const RpcEngine = require('json-rpc-engine') const createIdRemapMiddleware = require('json-rpc-engine/src/idRemapMiddleware') const createStreamMiddleware = require('json-rpc-middleware-stream') @@ -12,7 +12,7 @@ function MetamaskInpageProvider (connectionStream) { // setup connectionStream multiplexing var multiStream = self.multiStream = ObjectMultiplex() - pipe( + pump( connectionStream, multiStream, connectionStream, @@ -21,7 +21,7 @@ function MetamaskInpageProvider (connectionStream) { // subscribe to metamask public config (one-way) self.publicConfigStore = new LocalStorageStore({ storageKey: 'MetaMask-Config' }) - pipe( + pump( multiStream.createStream('publicConfig'), self.publicConfigStore, (err) => logStreamDisconnectWarning('MetaMask PublicConfigStore', err) @@ -32,7 +32,7 @@ function MetamaskInpageProvider (connectionStream) { // connect to async provider const streamMiddleware = createStreamMiddleware() - pipe( + pump( streamMiddleware.stream, multiStream.createStream('provider'), streamMiddleware.stream, From 9d4c02e57f2b147759d979d8a6c051aa008cdff0 Mon Sep 17 00:00:00 2001 From: kumavis Date: Thu, 7 Sep 2017 21:26:25 -0700 Subject: [PATCH 05/19] metamask - add jsonrpc filter middleware on per-connection engine --- app/scripts/metamask-controller.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 1a6732338..735fc4af0 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -8,6 +8,7 @@ const EthStore = require('./lib/eth-store') const EthQuery = require('eth-query') const RpcEngine = require('json-rpc-engine') const createEngineStream = require('json-rpc-middleware-stream/engineStream') +const createFilterMiddleware = require('eth-json-rpc-filters') const setupMultiplex = require('./lib/stream-utils.js').setupMultiplex const KeyringController = require('./keyring-controller') const NetworkController = require('./controllers/network') @@ -78,12 +79,13 @@ module.exports = class MetamaskController extends EventEmitter { // rpc provider this.provider = this.initializeProvider() + this.blockTracker = this.provider // eth data query tools this.ethQuery = new EthQuery(this.provider) this.ethStore = new EthStore({ provider: this.provider, - blockTracker: this.provider, + blockTracker: this.blockTracker, }) // key mgmt @@ -110,7 +112,7 @@ module.exports = class MetamaskController extends EventEmitter { getNetwork: this.networkController.getNetworkState.bind(this), signTransaction: this.keyringController.signTransaction.bind(this.keyringController), provider: this.provider, - blockTracker: this.provider, + blockTracker: this.blockTracker, ethQuery: this.ethQuery, ethStore: this.ethStore, }) @@ -387,6 +389,10 @@ module.exports = class MetamaskController extends EventEmitter { const engine = new RpcEngine() engine.push(originMiddleware) engine.push(loggerMiddleware) + engine.push(createFilterMiddleware({ + provider: this.provider, + blockTracker: this.blockTracker, + })) engine.push(createProviderMiddleware({ provider: this.provider })) // setup connection From f5d0a0b07ac835810bc6faef88384c2872846414 Mon Sep 17 00:00:00 2001 From: kumavis Date: Thu, 7 Sep 2017 22:25:08 -0700 Subject: [PATCH 06/19] deps - bump jsonrpc filters for log filter formate fix --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 295d33409..3fede273c 100644 --- a/package.json +++ b/package.json @@ -69,7 +69,7 @@ "eth-bin-to-ops": "^1.0.1", "eth-contract-metadata": "^1.1.4", "eth-hd-keyring": "^1.1.1", - "eth-json-rpc-filters": "^1.0.1", + "eth-json-rpc-filters": "^1.0.2", "eth-phishing-detect": "^1.1.4", "eth-query": "^2.1.2", "eth-sig-util": "^1.2.2", From 70401626e2544f254ddb06a21b5d3de7fdd7fb82 Mon Sep 17 00:00:00 2001 From: kumavis Date: Thu, 7 Sep 2017 22:35:38 -0700 Subject: [PATCH 07/19] lint - remove dead code --- app/scripts/lib/stream-utils.js | 1 - 1 file changed, 1 deletion(-) diff --git a/app/scripts/lib/stream-utils.js b/app/scripts/lib/stream-utils.js index 89e2a359e..8bb0b4f3c 100644 --- a/app/scripts/lib/stream-utils.js +++ b/app/scripts/lib/stream-utils.js @@ -1,5 +1,4 @@ const Through = require('through2') -const endOfStream = require('end-of-stream') const ObjectMultiplex = require('obj-multiplex') const pump = require('pump') From ef3bf810bf14da6651ef849e481eb0253be3c8d1 Mon Sep 17 00:00:00 2001 From: kumavis Date: Thu, 7 Sep 2017 22:47:08 -0700 Subject: [PATCH 08/19] inpage - use obj-multiplex module --- app/scripts/lib/inpage-provider.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/app/scripts/lib/inpage-provider.js b/app/scripts/lib/inpage-provider.js index db46e4f17..b2515bfb8 100644 --- a/app/scripts/lib/inpage-provider.js +++ b/app/scripts/lib/inpage-provider.js @@ -3,7 +3,7 @@ const RpcEngine = require('json-rpc-engine') const createIdRemapMiddleware = require('json-rpc-engine/src/idRemapMiddleware') const createStreamMiddleware = require('json-rpc-middleware-stream') const LocalStorageStore = require('obs-store') -const ObjectMultiplex = require('./obj-multiplex') +const ObjectMultiplex = require('obj-multiplex') module.exports = MetamaskInpageProvider @@ -11,10 +11,10 @@ function MetamaskInpageProvider (connectionStream) { const self = this // setup connectionStream multiplexing - var multiStream = self.multiStream = ObjectMultiplex() + const mux = self.mux = new ObjectMultiplex() pump( connectionStream, - multiStream, + mux, connectionStream, (err) => logStreamDisconnectWarning('MetaMask', err) ) @@ -22,19 +22,19 @@ function MetamaskInpageProvider (connectionStream) { // subscribe to metamask public config (one-way) self.publicConfigStore = new LocalStorageStore({ storageKey: 'MetaMask-Config' }) pump( - multiStream.createStream('publicConfig'), + mux.createStream('publicConfig'), self.publicConfigStore, (err) => logStreamDisconnectWarning('MetaMask PublicConfigStore', err) ) // ignore phishing warning message (handled elsewhere) - multiStream.ignoreStream('phishing') + mux.ignoreStream('phishing') // connect to async provider const streamMiddleware = createStreamMiddleware() pump( streamMiddleware.stream, - multiStream.createStream('provider'), + mux.createStream('provider'), streamMiddleware.stream, (err) => logStreamDisconnectWarning('MetaMask RpcProvider', err) ) From 8545453a9d58d7a54c10beef52e38107ed937117 Mon Sep 17 00:00:00 2001 From: kumavis Date: Mon, 11 Sep 2017 14:30:30 -0700 Subject: [PATCH 09/19] contentscript - fix obj-multiplex instantiation and use pump for streams --- app/scripts/contentscript.js | 45 ++++++++++++++++++++++++++---------- 1 file changed, 33 insertions(+), 12 deletions(-) diff --git a/app/scripts/contentscript.js b/app/scripts/contentscript.js index acacf5d4c..90a0f1f22 100644 --- a/app/scripts/contentscript.js +++ b/app/scripts/contentscript.js @@ -1,11 +1,12 @@ +const fs = require('fs') +const path = require('path') +const pump = require('pump') const LocalMessageDuplexStream = require('post-message-stream') const PongStream = require('ping-pong-stream/pong') -const PortStream = require('./lib/port-stream.js') -const ObjectMultiplex = require('./lib/obj-multiplex') +const ObjectMultiplex = require('obj-multiplex') const extension = require('extensionizer') +const PortStream = require('./lib/port-stream.js') -const fs = require('fs') -const path = require('path') const inpageText = fs.readFileSync(path.join(__dirname, 'inpage.js')).toString() // Eventually this streaming injection could be replaced with: @@ -50,22 +51,42 @@ function setupStreams () { pageStream.pipe(pluginStream).pipe(pageStream) // setup local multistream channels - const mx = ObjectMultiplex() - mx.on('error', console.error) - mx.pipe(pageStream).pipe(mx) - mx.pipe(pluginStream).pipe(mx) + const mux = new ObjectMultiplex() + pump( + mux, + pageStream, + mux, + (err) => logStreamDisconnectWarning('MetaMask Inpage', err) + ) + pump( + mux, + pluginStream, + mux, + (err) => logStreamDisconnectWarning('MetaMask Background', err) + ) // connect ping stream const pongStream = new PongStream({ objectMode: true }) - pongStream.pipe(mx.createStream('pingpong')).pipe(pongStream) + pump( + mux, + pongStream, + mux, + (err) => logStreamDisconnectWarning('MetaMask PingPongStream', err) + ) // connect phishing warning stream - const phishingStream = mx.createStream('phishing') + const phishingStream = mux.createStream('phishing') phishingStream.once('data', redirectToPhishingWarning) // ignore unused channels (handled by background, inpage) - mx.ignoreStream('provider') - mx.ignoreStream('publicConfig') + mux.ignoreStream('provider') + mux.ignoreStream('publicConfig') +} + +function logStreamDisconnectWarning (remoteLabel, err) { + let warningMsg = `MetamaskContentscript - lost connection to ${remoteLabel}` + if (err) warningMsg += '\n' + err.stack + console.warn(warningMsg) } function shouldInjectWeb3 () { From a265144176658220c5d8279ecb18c3ac0810e2c2 Mon Sep 17 00:00:00 2001 From: kumavis Date: Wed, 13 Sep 2017 10:21:00 -0700 Subject: [PATCH 10/19] metamask cont - standardize multiplex stream naming --- app/scripts/metamask-controller.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 735fc4af0..b28f5042a 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -346,23 +346,23 @@ module.exports = class MetamaskController extends EventEmitter { } // setup multiplexing - const mx = setupMultiplex(connectionStream) + const mux = setupMultiplex(connectionStream) // connect features - this.setupProviderConnection(mx.createStream('provider'), originDomain) - this.setupPublicConfig(mx.createStream('publicConfig')) + this.setupProviderConnection(mux.createStream('provider'), originDomain) + this.setupPublicConfig(mux.createStream('publicConfig')) } setupTrustedCommunication (connectionStream, originDomain) { // setup multiplexing - const mx = setupMultiplex(connectionStream) + const mux = setupMultiplex(connectionStream) // connect features - this.setupControllerConnection(mx.createStream('controller')) - this.setupProviderConnection(mx.createStream('provider'), originDomain) + this.setupControllerConnection(mux.createStream('controller')) + this.setupProviderConnection(mux.createStream('provider'), originDomain) } sendPhishingWarning (connectionStream, hostname) { - const mx = setupMultiplex(connectionStream) - const phishingStream = mx.createStream('phishing') + const mux = setupMultiplex(connectionStream) + const phishingStream = mux.createStream('phishing') phishingStream.write({ hostname }) } From 96d1175834bb5d400f6a70d228cebbd23bded4db Mon Sep 17 00:00:00 2001 From: kumavis Date: Wed, 13 Sep 2017 10:28:29 -0700 Subject: [PATCH 11/19] debug - prefer logger over console --- app/scripts/background.js | 8 ++++---- app/scripts/metamask-controller.js | 10 +++++----- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/app/scripts/background.js b/app/scripts/background.js index f077ca7a8..1b96d68b5 100644 --- a/app/scripts/background.js +++ b/app/scripts/background.js @@ -1,6 +1,8 @@ const urlUtil = require('url') const endOfStream = require('end-of-stream') const pipe = require('pump') +const log = require('loglevel') +const extension = require('extensionizer') const LocalStorageStore = require('obs-store/lib/localStorage') const storeTransform = require('obs-store/lib/transform') const ExtensionPlatform = require('./platforms/extension') @@ -9,13 +11,11 @@ const migrations = require('./migrations/') const PortStream = require('./lib/port-stream.js') const NotificationManager = require('./lib/notification-manager.js') const MetamaskController = require('./metamask-controller') -const extension = require('extensionizer') const firstTimeState = require('./first-time-state') const STORAGE_KEY = 'metamask-config' const METAMASK_DEBUG = 'GULP_METAMASK_DEBUG' -const log = require('loglevel') window.log = log log.setDefaultLevel(METAMASK_DEBUG ? 'debug' : 'warn') @@ -29,12 +29,12 @@ let popupIsOpen = false const diskStore = new LocalStorageStore({ storageKey: STORAGE_KEY }) // initialization flow -initialize().catch(console.error) +initialize().catch(log.error) async function initialize () { const initState = await loadStateFromPersistence() await setupController(initState) - console.log('MetaMask initialization complete.') + log.debug('MetaMask initialization complete.') } // diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index b28f5042a..0c9602568 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -340,7 +340,7 @@ module.exports = class MetamaskController extends EventEmitter { setupUntrustedCommunication (connectionStream, originDomain) { // Check if new connection is blacklisted if (this.blacklistController.checkForPhishing(originDomain)) { - console.log('MetaMask - sending phishing warning for', originDomain) + log.debug('MetaMask - sending phishing warning for', originDomain) this.sendPhishingWarning(connectionStream, originDomain) return } @@ -374,7 +374,7 @@ module.exports = class MetamaskController extends EventEmitter { dnode, outStream, (err) => { - if (err) console.error(err) + if (err) log.error(err) } ) dnode.on('remote', (remote) => { @@ -402,7 +402,7 @@ module.exports = class MetamaskController extends EventEmitter { providerStream, outStream, (err) => { - if (err) console.error(err) + if (err) log.error(err) } ) @@ -416,7 +416,7 @@ module.exports = class MetamaskController extends EventEmitter { function loggerMiddleware (req, res, next, end) { next((cb) => { if (res.error) { - console.error('Error in RPC response:\n', res) + log.error('Error in RPC response:\n', res) } if (req.isMetamaskInternal) return log.info(`RPC (${originDomain}):`, req, '->', res) @@ -441,7 +441,7 @@ module.exports = class MetamaskController extends EventEmitter { this.publicConfigStore, outStream, (err) => { - if (err) console.error(err) + if (err) log.error(err) } ) } From 06153dd47d16c2e3c6eed7470501d7534c29cbd4 Mon Sep 17 00:00:00 2001 From: Kevin Serrano Date: Wed, 13 Sep 2017 12:17:42 -0700 Subject: [PATCH 12/19] Add warning of higher failure risk since app proposed gasLimit. --- ui/app/components/pending-tx.js | 64 ++++++++++++++++++++------------- 1 file changed, 39 insertions(+), 25 deletions(-) diff --git a/ui/app/components/pending-tx.js b/ui/app/components/pending-tx.js index 3e53d47f9..37b242728 100644 --- a/ui/app/components/pending-tx.js +++ b/ui/app/components/pending-tx.js @@ -52,7 +52,8 @@ PendingTx.prototype.render = function () { const gas = txParams.gas const gasBn = hexToBn(gas) const gasLimit = new BN(parseInt(blockGasLimit)) - const safeGasLimit = this.bnMultiplyByFraction(gasLimit, 19, 20).toString(10) + const safeGasLimitBN = this.bnMultiplyByFraction(gasLimit, 19, 20) + const safeGasLimit = safeGasLimitBN.toString(10) // Gas Price const gasPrice = txParams.gasPrice || MIN_GAS_PRICE_BN.toString(16) @@ -66,6 +67,8 @@ PendingTx.prototype.render = function () { const balanceBn = hexToBn(balance) const insufficientBalance = balanceBn.lt(maxCost) + const dangerousGasLimit = gasBn.gte(safeGasLimitBN) + const gasLimitSpecified = txMeta.gasLimitSpecified const buyDisabled = insufficientBalance || !this.state.valid || !isValidAddress || this.state.submitting const showRejectAll = props.unconfTxListLength > 1 @@ -263,33 +266,44 @@ PendingTx.prototype.render = function () { text-transform: uppercase; } `), + h('.cell.row', { + style: { + textAlign: 'center', + }, + }, [ + txMeta.simulationFails ? + h('.error', { + style: { + fontSize: '0.9em', + }, + }, 'Transaction Error. Exception thrown in contract code.') + : null, - txMeta.simulationFails ? - h('.error', { - style: { - marginLeft: 50, - fontSize: '0.9em', - }, - }, 'Transaction Error. Exception thrown in contract code.') - : null, + !isValidAddress ? + h('.error', { + style: { + fontSize: '0.9em', + }, + }, 'Recipient address is invalid. Sending this transaction will result in a loss of ETH.') + : null, - !isValidAddress ? - h('.error', { - style: { - marginLeft: 50, - fontSize: '0.9em', - }, - }, 'Recipient address is invalid. Sending this transaction will result in a loss of ETH.') - : null, + insufficientBalance ? + h('span.error', { + style: { + fontSize: '0.9em', + }, + }, 'Insufficient balance for transaction') + : null, + + (dangerousGasLimit && gasLimitSpecified) ? + h('span.error', { + style: { + fontSize: '0.9em', + }, + }, 'Gas limit set dangerously high. Approving this transaction is likely to fail.') + : null, + ]), - insufficientBalance ? - h('span.error', { - style: { - marginLeft: 50, - fontSize: '0.9em', - }, - }, 'Insufficient balance for transaction') - : null, // send + cancel h('.flex-row.flex-space-around.conf-buttons', { From 6e725b123b878da12494904b121237ee41af7f76 Mon Sep 17 00:00:00 2001 From: Kevin Serrano Date: Wed, 13 Sep 2017 12:22:08 -0700 Subject: [PATCH 13/19] Lower warning threshold for high tx fee to account for fluctuating blockGasLimits --- ui/app/components/pending-tx.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ui/app/components/pending-tx.js b/ui/app/components/pending-tx.js index 37b242728..ec2b15de4 100644 --- a/ui/app/components/pending-tx.js +++ b/ui/app/components/pending-tx.js @@ -53,6 +53,7 @@ PendingTx.prototype.render = function () { const gasBn = hexToBn(gas) const gasLimit = new BN(parseInt(blockGasLimit)) const safeGasLimitBN = this.bnMultiplyByFraction(gasLimit, 19, 20) + const saferGasLimitBN = this.bnMultiplyByFraction(gasLimit, 18, 20) const safeGasLimit = safeGasLimitBN.toString(10) // Gas Price @@ -67,7 +68,7 @@ PendingTx.prototype.render = function () { const balanceBn = hexToBn(balance) const insufficientBalance = balanceBn.lt(maxCost) - const dangerousGasLimit = gasBn.gte(safeGasLimitBN) + const dangerousGasLimit = gasBn.gte(saferGasLimitBN) const gasLimitSpecified = txMeta.gasLimitSpecified const buyDisabled = insufficientBalance || !this.state.valid || !isValidAddress || this.state.submitting const showRejectAll = props.unconfTxListLength > 1 From 3d36d565afe908ca7c4424ff8a7d3659d0978aca Mon Sep 17 00:00:00 2001 From: Kevin Serrano Date: Wed, 13 Sep 2017 12:26:24 -0700 Subject: [PATCH 14/19] Bump changelog. --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f30c04985..6f357c92c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ - Add ability to export seed words as a file. - Changed state logs to a file download than a clipboard copy. - Fix link to support center. +- Warn users when a dapp proposes a high gas limit (90% of blockGasLimit or higher) ## 3.10.0 2017-9-11 From a22a2586abcca35d2a4bc1a0407aeaacc55f0a27 Mon Sep 17 00:00:00 2001 From: Kevin Serrano Date: Wed, 13 Sep 2017 13:24:16 -0700 Subject: [PATCH 15/19] Haha silly me, only when gas is estimated and not explicit. --- ui/app/components/pending-tx.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/app/components/pending-tx.js b/ui/app/components/pending-tx.js index ec2b15de4..c3350fcc1 100644 --- a/ui/app/components/pending-tx.js +++ b/ui/app/components/pending-tx.js @@ -296,7 +296,7 @@ PendingTx.prototype.render = function () { }, 'Insufficient balance for transaction') : null, - (dangerousGasLimit && gasLimitSpecified) ? + (dangerousGasLimit && !gasLimitSpecified) ? h('span.error', { style: { fontSize: '0.9em', From 245c0f0c2741d1dcb706faa93ff681333a40b9c8 Mon Sep 17 00:00:00 2001 From: kumavis Date: Wed, 13 Sep 2017 15:17:26 -0700 Subject: [PATCH 16/19] metamask controller - move middleware into seperate files --- app/scripts/lib/createLoggerMiddleware.js | 15 ++++++++ app/scripts/lib/createOriginMiddleware.js | 9 +++++ app/scripts/lib/createProviderMiddleware.js | 13 +++++++ app/scripts/metamask-controller.js | 41 ++++----------------- package.json | 2 +- 5 files changed, 45 insertions(+), 35 deletions(-) create mode 100644 app/scripts/lib/createLoggerMiddleware.js create mode 100644 app/scripts/lib/createOriginMiddleware.js create mode 100644 app/scripts/lib/createProviderMiddleware.js diff --git a/app/scripts/lib/createLoggerMiddleware.js b/app/scripts/lib/createLoggerMiddleware.js new file mode 100644 index 000000000..b92a965de --- /dev/null +++ b/app/scripts/lib/createLoggerMiddleware.js @@ -0,0 +1,15 @@ +// log rpc activity +module.exports = createLoggerMiddleware + +function createLoggerMiddleware({ origin }) { + return function loggerMiddleware (req, res, next, end) { + next((cb) => { + if (res.error) { + log.error('Error in RPC response:\n', res) + } + if (req.isMetamaskInternal) return + log.info(`RPC (${origin}):`, req, '->', res) + cb() + }) + } +} \ No newline at end of file diff --git a/app/scripts/lib/createOriginMiddleware.js b/app/scripts/lib/createOriginMiddleware.js new file mode 100644 index 000000000..f21d79512 --- /dev/null +++ b/app/scripts/lib/createOriginMiddleware.js @@ -0,0 +1,9 @@ +// append dapp origin domain to request +module.exports = createOriginMiddleware + +function createOriginMiddleware({ origin }) { + return function originMiddleware (req, res, next, end) { + req.origin = originDomain + next() + } +} \ No newline at end of file diff --git a/app/scripts/lib/createProviderMiddleware.js b/app/scripts/lib/createProviderMiddleware.js new file mode 100644 index 000000000..6dd192411 --- /dev/null +++ b/app/scripts/lib/createProviderMiddleware.js @@ -0,0 +1,13 @@ + +module.exports = createProviderMiddleware + +// forward requests to provider +function createProviderMiddleware({ provider }) { + return (req, res, next, end) => { + provider.sendAsync(req, (err, _res) => { + if (err) return end(err) + res.result = _res.result + end() + }) + } +} \ No newline at end of file diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 0c9602568..f114d22f3 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -7,8 +7,12 @@ const ObservableStore = require('obs-store') const EthStore = require('./lib/eth-store') const EthQuery = require('eth-query') const RpcEngine = require('json-rpc-engine') +const debounce = require('debounce') const createEngineStream = require('json-rpc-middleware-stream/engineStream') const createFilterMiddleware = require('eth-json-rpc-filters') +const createOriginMiddleware = require('./lib/createOriginMiddleware') +const createLoggerMiddleware = require('./lib/createLoggerMiddleware') +const createProviderMiddleware = require('./lib/createProviderMiddleware') const setupMultiplex = require('./lib/stream-utils.js').setupMultiplex const KeyringController = require('./keyring-controller') const NetworkController = require('./controllers/network') @@ -26,8 +30,6 @@ const ConfigManager = require('./lib/config-manager') const nodeify = require('./lib/nodeify') const accountImporter = require('./account-import-strategies') const getBuyEthUrl = require('./lib/buy-eth-url') -const debounce = require('debounce') - const version = require('../manifest.json').version module.exports = class MetamaskController extends EventEmitter { @@ -384,11 +386,11 @@ module.exports = class MetamaskController extends EventEmitter { }) } - setupProviderConnection (outStream, originDomain) { + setupProviderConnection (outStream, origin) { // setup json rpc engine stack const engine = new RpcEngine() - engine.push(originMiddleware) - engine.push(loggerMiddleware) + engine.push(createOriginMiddleware({ origin })) + engine.push(createLoggerMiddleware({ origin })) engine.push(createFilterMiddleware({ provider: this.provider, blockTracker: this.blockTracker, @@ -405,35 +407,6 @@ module.exports = class MetamaskController extends EventEmitter { if (err) log.error(err) } ) - - // append dapp origin domain to request - function originMiddleware (req, res, next, end) { - req.origin = originDomain - next() - } - - // log rpc activity - function loggerMiddleware (req, res, next, end) { - next((cb) => { - if (res.error) { - log.error('Error in RPC response:\n', res) - } - if (req.isMetamaskInternal) return - log.info(`RPC (${originDomain}):`, req, '->', res) - cb() - }) - } - - // forward requests to provider - function createProviderMiddleware({ provider }) { - return (req, res, next, end) => { - provider.sendAsync(req, (err, _res) => { - if (err) return end(err) - res.result = _res.result - end() - }) - } - } } setupPublicConfig (outStream) { diff --git a/package.json b/package.json index b241ccfc6..3f9d9c538 100644 --- a/package.json +++ b/package.json @@ -68,7 +68,7 @@ "eth-bin-to-ops": "^1.0.1", "eth-contract-metadata": "^1.1.4", "eth-hd-keyring": "^1.1.1", - "eth-json-rpc-filters": "^1.0.2", + "eth-json-rpc-filters": "^1.1.0", "eth-phishing-detect": "^1.1.4", "eth-query": "^2.1.2", "eth-sig-util": "^1.2.2", From 765ef640610584a3fdd7fa0ef716f6c7a553407c Mon Sep 17 00:00:00 2001 From: kumavis Date: Wed, 13 Sep 2017 15:19:44 -0700 Subject: [PATCH 17/19] metamask controller - destroy filter polyfill on disconnect --- app/scripts/metamask-controller.js | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index f114d22f3..fef16c3a9 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -389,12 +389,16 @@ module.exports = class MetamaskController extends EventEmitter { setupProviderConnection (outStream, origin) { // setup json rpc engine stack const engine = new RpcEngine() - engine.push(createOriginMiddleware({ origin })) - engine.push(createLoggerMiddleware({ origin })) - engine.push(createFilterMiddleware({ + + // create filter polyfill middleware + const filterMiddleware = createFilterMiddleware({ provider: this.provider, blockTracker: this.blockTracker, - })) + }) + + engine.push(createOriginMiddleware({ origin })) + engine.push(createLoggerMiddleware({ origin })) + engine.push(filterMiddleware) engine.push(createProviderMiddleware({ provider: this.provider })) // setup connection @@ -404,6 +408,8 @@ module.exports = class MetamaskController extends EventEmitter { providerStream, outStream, (err) => { + // cleanup filter polyfill middleware + filterMiddleware.destroy() if (err) log.error(err) } ) From 6ba448edaf64d1122ab0b5c8af21f9806164c309 Mon Sep 17 00:00:00 2001 From: kumavis Date: Wed, 13 Sep 2017 15:26:00 -0700 Subject: [PATCH 18/19] changelog - add note of filter memory leak fix --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 89eaefa79..e3cf3505a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ - Add ability to export private keys as a file. - Add ability to export seed words as a file. - Changed state logs to a file download than a clipboard copy. +- Fixed a long standing memory leak associated with filters installed by dapps ## 3.10.0 2017-9-11 From d7097db0221cf6b6958ceb65e9ce4c893a8dfb61 Mon Sep 17 00:00:00 2001 From: kumavis Date: Wed, 13 Sep 2017 15:29:44 -0700 Subject: [PATCH 19/19] createOriginMiddleware - fix var name --- app/scripts/lib/createOriginMiddleware.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/scripts/lib/createOriginMiddleware.js b/app/scripts/lib/createOriginMiddleware.js index f21d79512..e1e097cc4 100644 --- a/app/scripts/lib/createOriginMiddleware.js +++ b/app/scripts/lib/createOriginMiddleware.js @@ -3,7 +3,7 @@ module.exports = createOriginMiddleware function createOriginMiddleware({ origin }) { return function originMiddleware (req, res, next, end) { - req.origin = originDomain + req.origin = origin next() } } \ No newline at end of file