diff --git a/CHANGELOG.md b/CHANGELOG.md index 4fb81c350..5928d5cb4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,8 +5,10 @@ - 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 - Fix link to support center. - Fixed tooltip icon locations to avoid overflow. +- Warn users when a dapp proposes a high gas limit (90% of blockGasLimit or higher) ## 3.10.0 2017-9-11 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/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 () { 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..e1e097cc4 --- /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 = origin + 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/lib/inpage-provider.js b/app/scripts/lib/inpage-provider.js index 13888dc67..da75c4be2 100644 --- a/app/scripts/lib/inpage-provider.js +++ b/app/scripts/lib/inpage-provider.js @@ -1,8 +1,9 @@ -const pipe = require('pump') -const StreamProvider = require('web3-stream-provider') +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') const LocalStorageStore = require('obs-store') -const ObjectMultiplex = require('./obj-multiplex') -const createRandomId = require('./random-id') +const ObjectMultiplex = require('obj-multiplex') module.exports = MetamaskInpageProvider @@ -10,64 +11,46 @@ function MetamaskInpageProvider (connectionStream) { const self = this // setup connectionStream multiplexing - var multiStream = self.multiStream = ObjectMultiplex() - pipe( + const mux = self.mux = new ObjectMultiplex() + pump( connectionStream, - multiStream, + mux, connectionStream, (err) => logStreamDisconnectWarning('MetaMask', err) ) // subscribe to metamask public config (one-way) self.publicConfigStore = new LocalStorageStore({ storageKey: 'MetaMask-Config' }) - pipe( - multiStream.createStream('publicConfig'), + pump( + 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 asyncProvider = self.asyncProvider = new StreamProvider() - pipe( - asyncProvider, - multiStream.createStream('provider'), - asyncProvider, + const streamMiddleware = createStreamMiddleware() + pump( + streamMiddleware.stream, + mux.createStream('provider'), + streamMiddleware.stream, (err) => logStreamDisconnectWarning('MetaMask RpcProvider', err) ) - // start and stop polling to unblock first block lock - self.idMap = {} + // handle sendAsync requests via dapp-side rpc engine + const rpcEngine = new RpcEngine() + rpcEngine.push(createIdRemapMiddleware()) + rpcEngine.push(streamMiddleware) + self.rpcEngine = rpcEngine } // handle sendAsync requests via asyncProvider // also remap ids inbound and outbound MetamaskInpageProvider.prototype.sendAsync = function (payload, cb) { const self = this - - // rewrite request ids - const request = eachJsonMessage(payload, (_message) => { - const message = Object.assign({}, _message) - const newId = createRandomId() - self.idMap[newId] = message.id - message.id = newId - return message - }) - - // forward to asyncProvider - self.asyncProvider.sendAsync(request, (err, _res) => { - if (err) return cb(err) - // transform messages to original ids - const res = eachJsonMessage(_res, (message) => { - const oldId = self.idMap[message.id] - delete self.idMap[message.id] - message.id = oldId - return message - }) - cb(null, res) - }) + self.rpcEngine.handle(payload, cb) } @@ -124,14 +107,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/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..8bb0b4f3c 100644 --- a/app/scripts/lib/stream-utils.js +++ b/app/scripts/lib/stream-utils.js @@ -1,6 +1,6 @@ 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 +23,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 a007d6fc5..fef16c3a9 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -1,12 +1,18 @@ 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') const EthQuery = require('eth-query') -const streamIntoProvider = require('web3-stream-provider/handler') +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') @@ -24,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 { @@ -77,12 +81,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 @@ -109,7 +114,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, }) @@ -337,36 +342,43 @@ 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 } // 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 }) } setupControllerConnection (outStream) { const api = this.getApi() const dnode = Dnode(api) - outStream.pipe(dnode).pipe(outStream) + pump( + outStream, + dnode, + outStream, + (err) => { + if (err) log.error(err) + } + ) dnode.on('remote', (remote) => { // push updates to popup const sendUpdate = remote.sendUpdate.bind(remote) @@ -374,27 +386,42 @@ module.exports = class MetamaskController extends EventEmitter { }) } - setupProviderConnection (outStream, originDomain) { - streamIntoProvider(outStream, this.provider, onRequest, onResponse) - // append dapp origin domain to request - function onRequest (request) { - request.origin = originDomain - } - // 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) + setupProviderConnection (outStream, origin) { + // setup json rpc engine stack + const engine = new RpcEngine() + + // 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 + const providerStream = createEngineStream({ engine }) + pump( + outStream, + providerStream, + outStream, + (err) => { + // cleanup filter polyfill middleware + filterMiddleware.destroy() + if (err) log.error(err) } - if (request.isMetamaskInternal) return - log.info(`RPC (${originDomain}):`, request, '->', response) - } + ) } setupPublicConfig (outStream) { - pipe( + pump( this.publicConfigStore, - outStream + outStream, + (err) => { + if (err) log.error(err) + } ) } diff --git a/package.json b/package.json index 12f79ba35..3f9d9c538 100644 --- a/package.json +++ b/package.json @@ -68,6 +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.1.0", "eth-phishing-detect": "^1.1.4", "eth-query": "^2.1.2", "eth-sig-util": "^1.2.2", @@ -92,12 +93,15 @@ "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", "mississippi": "^1.2.0", "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", @@ -118,7 +122,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", diff --git a/ui/app/components/pending-tx.js b/ui/app/components/pending-tx.js index 3e53d47f9..c3350fcc1 100644 --- a/ui/app/components/pending-tx.js +++ b/ui/app/components/pending-tx.js @@ -52,7 +52,9 @@ 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 saferGasLimitBN = this.bnMultiplyByFraction(gasLimit, 18, 20) + const safeGasLimit = safeGasLimitBN.toString(10) // Gas Price const gasPrice = txParams.gasPrice || MIN_GAS_PRICE_BN.toString(16) @@ -66,6 +68,8 @@ PendingTx.prototype.render = function () { const balanceBn = hexToBn(balance) const insufficientBalance = balanceBn.lt(maxCost) + const dangerousGasLimit = gasBn.gte(saferGasLimitBN) + const gasLimitSpecified = txMeta.gasLimitSpecified const buyDisabled = insufficientBalance || !this.state.valid || !isValidAddress || this.state.submitting const showRejectAll = props.unconfTxListLength > 1 @@ -263,33 +267,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', {