From 82aa0d48d4e98c36bc1d90800d439a938b51ab03 Mon Sep 17 00:00:00 2001 From: "greenkeeper[bot]" Date: Tue, 18 Jul 2017 22:41:30 +0000 Subject: [PATCH 01/38] chore(package): update dependencies --- package.json | 64 ++++++++++++++++++++++++++-------------------------- 1 file changed, 32 insertions(+), 32 deletions(-) diff --git a/package.json b/package.json index d40ad068b..2e66080d6 100644 --- a/package.json +++ b/package.json @@ -46,7 +46,7 @@ ] }, "dependencies": { - "async": "^1.5.2", + "async": "^2.5.0", "await-semaphore": "^0.1.1", "babel-runtime": "^6.23.0", "bip39": "^2.2.0", @@ -54,10 +54,10 @@ "browser-passworder": "^2.0.3", "browserify-derequire": "^0.9.4", "client-sw-ready-event": "^3.3.0", - "clone": "^1.0.2", - "copy-to-clipboard": "^2.0.0", + "clone": "^2.1.1", + "copy-to-clipboard": "^3.0.6", "debounce": "^1.0.0", - "deep-extend": "^0.4.1", + "deep-extend": "^0.5.0", "detect-node": "^2.0.3", "disc": "^1.3.2", "dnode": "^1.2.2", @@ -78,12 +78,12 @@ "express": "^4.14.0", "extension-link-enabler": "^1.0.0", "extensionizer": "^1.0.0", - "gulp-eslint": "^2.0.0", + "gulp-eslint": "^4.0.0", "hat": "0.0.3", - "idb-global": "^1.0.0", - "identicon.js": "^1.2.1", + "idb-global": "^2.1.0", + "identicon.js": "^2.3.1", "iframe": "^1.0.0", - "iframe-stream": "^1.0.2", + "iframe-stream": "^3.0.0", "inject-css": "^0.1.1", "jazzicon": "^1.2.0", "loglevel": "^1.4.1", @@ -98,7 +98,7 @@ "ping-pong-stream": "^1.0.0", "pojo-migrator": "^2.1.0", "polyfill-crypto.getrandomvalues": "^1.0.0", - "post-message-stream": "^1.0.0", + "post-message-stream": "^3.0.0", "promise-filter": "^1.1.0", "promise-to-callback": "^1.0.0", "pump": "^1.0.2", @@ -107,33 +107,33 @@ "react": "^15.0.2", "react-addons-css-transition-group": "^15.0.2", "react-dom": "^15.5.4", - "react-hyperscript": "^2.2.2", + "react-hyperscript": "^3.0.0", "react-markdown": "^2.3.0", - "react-redux": "^4.4.5", + "react-redux": "^5.0.5", "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", "redux": "^3.0.5", - "redux-logger": "^2.10.2", - "redux-thunk": "^1.0.2", + "redux-logger": "^3.0.6", + "redux-thunk": "^2.2.0", "request-promise": "^4.1.1", "sandwich-expando": "^1.0.5", "semaphore": "^1.0.5", "sw-stream": "^2.0.0", "textarea-caret": "^3.0.1", - "three.js": "^0.73.2", + "three.js": "^0.77.1", "through2": "^2.0.1", "valid-url": "^1.0.9", "vreme": "^3.0.2", - "web3": "0.19.1", + "web3": "0.20.1", "web3-provider-engine": "^13.2.8", "web3-stream-provider": "^3.0.1", "xtend": "^4.0.1" }, "devDependencies": { "babel-core": "^6.24.1", - "babel-eslint": "^6.0.5", + "babel-eslint": "^7.2.3", "babel-plugin-transform-async-to-generator": "^6.24.1", "babel-plugin-transform-runtime": "^6.23.0", "babel-polyfill": "^6.23.0", @@ -142,45 +142,45 @@ "babelify": "^7.2.0", "beefy": "^2.1.5", "brfs": "^1.4.3", - "browserify": "^13.0.0", - "chai": "^3.5.0", + "browserify": "^14.4.0", + "chai": "^4.1.0", "deep-freeze-strict": "^1.1.1", - "del": "^2.2.0", + "del": "^3.0.0", "envify": "^4.0.0", "enzyme": "^2.8.2", "eslint-plugin-chai": "0.0.1", "eslint-plugin-mocha": "^4.9.0", - "fs-promise": "^1.0.0", + "fs-promise": "^2.0.3", "gulp": "github:gulpjs/gulp#4.0", "gulp-if": "^2.0.1", "gulp-json-editor": "^2.2.1", "gulp-livereload": "^3.8.1", - "gulp-replace": "^0.5.4", - "gulp-sourcemaps": "^1.6.0", + "gulp-replace": "^0.6.1", + "gulp-sourcemaps": "^2.6.0", "gulp-util": "^3.0.7", "gulp-watch": "^4.3.5", - "gulp-zip": "^3.2.0", + "gulp-zip": "^4.0.0", "isomorphic-fetch": "^2.2.1", - "jsdom": "^8.1.0", - "jsdom-global": "^1.7.0", - "jshint-stylish": "~0.1.5", + "jsdom": "^11.1.0", + "jsdom-global": "^3.0.2", + "jshint-stylish": "~2.2.1", "lodash.assign": "^4.0.6", - "mocha": "^2.4.5", - "mocha-eslint": "^2.1.1", + "mocha": "^3.4.2", + "mocha-eslint": "^4.0.0", "mocha-jsdom": "^1.1.0", - "mocha-sinon": "^1.1.5", - "nock": "^8.0.0", + "mocha-sinon": "^2.0.0", + "nock": "^9.0.14", "open": "0.0.5", "prompt": "^1.0.0", "qs": "^6.2.0", - "qunit": "^0.9.1", + "qunit": "^1.0.0", "react-addons-test-utils": "^15.5.1", "react-test-renderer": "^15.5.4", "react-testutils-additions": "^15.2.0", "sinon": "^2.3.8", "tape": "^4.5.1", "testem": "^1.10.3", - "uglifyify": "^3.0.1", + "uglifyify": "^4.0.2", "vinyl-buffer": "^1.0.0", "vinyl-source-stream": "^1.1.0", "watchify": "^3.7.0" From eff9a578f7c6f9435d7fabc3ec5efb7619b825d9 Mon Sep 17 00:00:00 2001 From: "greenkeeper[bot]" Date: Tue, 18 Jul 2017 22:41:33 +0000 Subject: [PATCH 02/38] docs(readme): add Greenkeeper badge --- README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.md b/README.md index 45fb68c78..90598d4c3 100644 --- a/README.md +++ b/README.md @@ -1,5 +1,7 @@ # MetaMask Plugin [![Build Status](https://circleci.com/gh/MetaMask/metamask-extension.svg?style=shield&circle-token=a1ddcf3cd38e29267f254c9c59d556d513e3a1fd)](https://circleci.com/gh/MetaMask/metamask-extension) +[![Greenkeeper badge](https://badges.greenkeeper.io/MetaMask/metamask-extension.svg)](https://greenkeeper.io/) + ## Developing Compatible Dapps If you're a web dapp developer, we've got two types of guides for you: From b471afcdb34cd121b9d3e3cccb3883943ba8aef5 Mon Sep 17 00:00:00 2001 From: frankiebee Date: Wed, 2 Aug 2017 19:24:34 -0400 Subject: [PATCH 03/38] use error for #approveTransaction when setting failed --- app/scripts/controllers/transactions.js | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index 720323e41..d4f32e049 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -229,11 +229,8 @@ module.exports = class TransactionController extends EventEmitter { // must set transaction to submitted/failed before releasing lock nonceLock.releaseLock() } catch (err) { - this.setTxStatusFailed(txId, { - stack: err.stack || err.message, - errCode: err.errCode || err, - message: err.message || 'Transaction failed during approval', - }) + if(!err.message) err.message = 'Transaction failed during approval' + this.setTxStatusFailed(txId, err) // must set transaction to submitted/failed before releasing lock if (nonceLock) nonceLock.releaseLock() // continue with error chain From da7471e0958d30b4465ef35090dbfa05b85471f0 Mon Sep 17 00:00:00 2001 From: kumavis Date: Thu, 3 Aug 2017 14:48:19 -0700 Subject: [PATCH 04/38] lint fixes --- .eslintrc | 4 ++-- app/scripts/contentscript.js | 2 +- ui/app/components/shift-list-item.js | 2 +- ui/app/components/transaction-list-item.js | 2 +- ui/app/reducers/app.js | 9 ++++++--- 5 files changed, 11 insertions(+), 8 deletions(-) diff --git a/.eslintrc b/.eslintrc index a57a93cdc..2eb0dc8b5 100644 --- a/.eslintrc +++ b/.eslintrc @@ -50,7 +50,7 @@ "eqeqeq": [2, "allow-null"], "generator-star-spacing": [2, { "before": true, "after": true }], "handle-callback-err": [1, "^(err|error)$" ], - "indent": [2, 2, { "SwitchCase": 1 }], + "indent": "off", "jsx-quotes": [2, "prefer-single"], "key-spacing": 1, "keyword-spacing": [2, { "before": true, "after": true }], @@ -133,7 +133,7 @@ "no-with": 2, "one-var": [2, { "initialized": "never" }], "operator-linebreak": [1, "after", { "overrides": { "?": "ignore", ":": "ignore" } }], - "padded-blocks": [1, "never"], + "padded-blocks": "off", "quotes": [2, "single", {"avoidEscape": true, "allowTemplateLiterals": true}], "semi": [2, "never"], "semi-spacing": [2, { "before": false, "after": true }], diff --git a/app/scripts/contentscript.js b/app/scripts/contentscript.js index 291b922e8..4b674dcc0 100644 --- a/app/scripts/contentscript.js +++ b/app/scripts/contentscript.js @@ -81,7 +81,7 @@ function suffixCheck () { var currentUrl = window.location.href var currentRegex for (let i = 0; i < prohibitedTypes.length; i++) { - currentRegex = new RegExp(`\.${prohibitedTypes[i]}$`) + currentRegex = new RegExp(`\\.${prohibitedTypes[i]}$`) if (currentRegex.test(currentUrl)) { return false } diff --git a/ui/app/components/shift-list-item.js b/ui/app/components/shift-list-item.js index 32bfbeda4..079f05e31 100644 --- a/ui/app/components/shift-list-item.js +++ b/ui/app/components/shift-list-item.js @@ -2,7 +2,7 @@ const inherits = require('util').inherits const Component = require('react').Component const h = require('react-hyperscript') const connect = require('react-redux').connect -const vreme = new (require('vreme')) +const vreme = new (require('vreme'))() const explorerLink = require('../../lib/explorer-link') const actions = require('../actions') const addressSummary = require('../util').addressSummary diff --git a/ui/app/components/transaction-list-item.js b/ui/app/components/transaction-list-item.js index dbda66a31..b94c24494 100644 --- a/ui/app/components/transaction-list-item.js +++ b/ui/app/components/transaction-list-item.js @@ -6,7 +6,7 @@ const EthBalance = require('./eth-balance') const addressSummary = require('../util').addressSummary const explorerLink = require('../../lib/explorer-link') const CopyButton = require('./copyButton') -const vreme = new (require('vreme')) +const vreme = new (require('vreme'))() const Tooltip = require('./tooltip') const numberToBN = require('number-to-bn') diff --git a/ui/app/reducers/app.js b/ui/app/reducers/app.js index 2fcc9bfe0..3a98d53a9 100644 --- a/ui/app/reducers/app.js +++ b/ui/app/reducers/app.js @@ -40,9 +40,12 @@ function reduceApp (state, action) { accountDetail: { subview: 'transactions', }, - transForward: true, // Used to render transition direction - isLoading: false, // Used to display loading indicator - warning: null, // Used to display error text + // Used to render transition direction + transForward: true, + // Used to display loading indicator + isLoading: false, + // Used to display error text + warning: null, }, state.appState) switch (action.type) { From da16f396266daa5ab0acc8f0c04d3e25b98c39f0 Mon Sep 17 00:00:00 2001 From: kumavis Date: Thu, 3 Aug 2017 15:05:32 -0700 Subject: [PATCH 05/38] Merge branch 'master' of github.com:MetaMask/metamask-extension into greenkeeper/initial --- .gitignore | 5 +- CHANGELOG.md | 12 + README.md | 14 +- app/manifest.json | 7 +- app/scripts/background.js | 10 +- app/scripts/blacklister.js | 13 - app/scripts/contentscript.js | 22 +- app/scripts/controllers/blacklist.js | 58 ++++ app/scripts/controllers/infura.js | 2 +- app/scripts/controllers/network.js | 5 +- app/scripts/controllers/transactions.js | 363 +++++++++++++----------- app/scripts/inpage.js | 1 + app/scripts/lib/inpage-provider.js | 3 + app/scripts/lib/nonce-tracker.js | 28 +- app/scripts/lib/obj-multiplex.js | 16 +- app/scripts/lib/tx-utils.js | 61 +--- app/scripts/lib/util.js | 8 + app/scripts/metamask-controller.js | 55 ++-- circle.yml | 3 + gulpfile.js | 1 - package.json | 17 +- test/unit/blacklist-controller-test.js | 41 +++ test/unit/network-contoller-test.js | 36 ++- test/unit/nonce-tracker-test.js | 8 +- test/unit/tx-controller-test.js | 170 +++++++++-- ui/app/components/pending-tx.js | 2 +- ui/app/reducers.js | 1 - 27 files changed, 611 insertions(+), 351 deletions(-) delete mode 100644 app/scripts/blacklister.js create mode 100644 app/scripts/controllers/blacklist.js create mode 100644 app/scripts/lib/util.js create mode 100644 test/unit/blacklist-controller-test.js diff --git a/.gitignore b/.gitignore index 85c2d15d6..1806b1932 100644 --- a/.gitignore +++ b/.gitignore @@ -24,4 +24,7 @@ test/background.js test/bundle.js test/test-bundle.js -notes.txt \ No newline at end of file +notes.txt + +.coveralls.yml +.nyc_output \ No newline at end of file diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d265d318..66c95a0c3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,11 +2,23 @@ ## Current Master +- Continuously update blacklist for known phishing sites in background. +- Automatically detect suspicious URLs too similar to common phishing targets, and blacklist them. + +## 3.9.2 2017-7-26 + +- Fix bugs that could sometimes result in failed transactions after switching networks. +- Include stack traces in txMeta's to better understand the life cycle of transactions +- Enhance blacklister functionality to include levenshtein logic. (credit to @sogoiii and @409H for their help!) + +## 3.9.1 2017-7-19 + - No longer automatically request 1 ropsten ether for the first account in a new vault. - Now redirects from known malicious sites faster. - Added a link to our new support page to the help screen. - Fixed bug where a new transaction would be shown over the current transaction, creating a possible timing attack against user confirmation. - Fixed bug in nonce tracker where an incorrect nonce would be calculated. +- Lowered minimum gas price to 1 Gwei. ## 3.9.0 2017-7-12 diff --git a/README.md b/README.md index 90598d4c3..c50216594 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,8 @@ -# MetaMask Plugin [![Build Status](https://circleci.com/gh/MetaMask/metamask-extension.svg?style=shield&circle-token=a1ddcf3cd38e29267f254c9c59d556d513e3a1fd)](https://circleci.com/gh/MetaMask/metamask-extension) +# MetaMask Plugin [![Build Status](https://circleci.com/gh/MetaMask/metamask-extension.svg?style=shield&circle-token=a1ddcf3cd38e29267f254c9c59d556d513e3a1fd)](https://circleci.com/gh/MetaMask/metamask-extension) [![Coverage Status](https://coveralls.io/repos/github/MetaMask/metamask-extension/badge.svg?branch=master)](https://coveralls.io/github/MetaMask/metamask-extension?branch=master) + +## Support + +If you're a user seeking support, [here is our support site](http://metamask.consensyssupport.happyfox.com). [![Greenkeeper badge](https://badges.greenkeeper.io/MetaMask/metamask-extension.svg)](https://greenkeeper.io/) @@ -6,7 +10,13 @@ If you're a web dapp developer, we've got two types of guides for you: -- If you've never built a Dapp before, we've got a gentle introduction on [Developing Dapps with Truffle and MetaMask](https://blog.metamask.io/developing-for-metamask-with-truffle/). +### New Dapp Developers + +- We recommend this [Learning Solidity](https://karl.tech/learning-solidity-part-1-deploy-a-contract/) tutorial series by Karl Floersch. +- We wrote a (slightly outdated now) gentle introduction on [Developing Dapps with Truffle and MetaMask](https://medium.com/metamask/developing-ethereum-dapps-with-truffle-and-metamask-aa8ad7e363ba). + +### Current Dapp Developers + - If you have a Dapp, and you want to ensure compatibility, [here is our guide on building MetaMask-compatible Dapps](https://github.com/MetaMask/faq/blob/master/DEVELOPERS.md) ## Building locally diff --git a/app/manifest.json b/app/manifest.json index 7bf757d4c..1eaf6f26a 100644 --- a/app/manifest.json +++ b/app/manifest.json @@ -1,7 +1,7 @@ { "name": "MetaMask", "short_name": "Metamask", - "version": "3.9.0", + "version": "3.9.2", "manifest_version": 2, "author": "https://metamask.io", "description": "Ethereum Browser Extension", @@ -52,11 +52,6 @@ ], "run_at": "document_start", "all_frames": true - }, - { - "run_at": "document_start", - "matches": ["http://*/*", "https://*/*"], - "js": ["scripts/blacklister.js"] } ], "permissions": [ diff --git a/app/scripts/background.js b/app/scripts/background.js index e8987394f..f077ca7a8 100644 --- a/app/scripts/background.js +++ b/app/scripts/background.js @@ -90,12 +90,12 @@ function setupController (initState) { extension.runtime.onConnect.addListener(connectRemote) function connectRemote (remotePort) { - var isMetaMaskInternalProcess = remotePort.name === 'popup' || remotePort.name === 'notification' - var portStream = new PortStream(remotePort) + const isMetaMaskInternalProcess = remotePort.name === 'popup' || remotePort.name === 'notification' + const portStream = new PortStream(remotePort) if (isMetaMaskInternalProcess) { // communication with popup popupIsOpen = popupIsOpen || (remotePort.name === 'popup') - controller.setupTrustedCommunication(portStream, 'MetaMask', remotePort.name) + controller.setupTrustedCommunication(portStream, 'MetaMask') // record popup as closed if (remotePort.name === 'popup') { endOfStream(portStream, () => { @@ -104,7 +104,7 @@ function setupController (initState) { } } else { // communication with page - var originDomain = urlUtil.parse(remotePort.sender.url).hostname + const originDomain = urlUtil.parse(remotePort.sender.url).hostname controller.setupUntrustedCommunication(portStream, originDomain) } } @@ -121,7 +121,7 @@ function setupController (initState) { // plugin badge text function updateBadge () { var label = '' - var unapprovedTxCount = controller.txController.unapprovedTxCount + var unapprovedTxCount = controller.txController.getUnapprovedTxCount() var unapprovedMsgCount = controller.messageManager.unapprovedMsgCount var unapprovedPersonalMsgs = controller.personalMessageManager.unapprovedPersonalMsgCount var count = unapprovedTxCount + unapprovedMsgCount + unapprovedPersonalMsgs diff --git a/app/scripts/blacklister.js b/app/scripts/blacklister.js deleted file mode 100644 index a45265a75..000000000 --- a/app/scripts/blacklister.js +++ /dev/null @@ -1,13 +0,0 @@ -const blacklistedDomains = require('etheraddresslookup/blacklists/domains.json') - -function detectBlacklistedDomain() { - var strCurrentTab = window.location.hostname - if (blacklistedDomains && blacklistedDomains.includes(strCurrentTab)) { - window.location.href = 'https://metamask.io/phishing.html' - } -} - -window.addEventListener('load', function() { - detectBlacklistedDomain() -}) - diff --git a/app/scripts/contentscript.js b/app/scripts/contentscript.js index 4b674dcc0..acacf5d4c 100644 --- a/app/scripts/contentscript.js +++ b/app/scripts/contentscript.js @@ -37,28 +37,33 @@ function setupInjection () { function setupStreams () { // setup communication to page and plugin - var pageStream = new LocalMessageDuplexStream({ + const pageStream = new LocalMessageDuplexStream({ name: 'contentscript', target: 'inpage', }) pageStream.on('error', console.error) - var pluginPort = extension.runtime.connect({name: 'contentscript'}) - var pluginStream = new PortStream(pluginPort) + const pluginPort = extension.runtime.connect({ name: 'contentscript' }) + const pluginStream = new PortStream(pluginPort) pluginStream.on('error', console.error) // forward communication plugin->inpage pageStream.pipe(pluginStream).pipe(pageStream) // setup local multistream channels - var mx = ObjectMultiplex() + const mx = ObjectMultiplex() mx.on('error', console.error) mx.pipe(pageStream).pipe(mx) + mx.pipe(pluginStream).pipe(mx) // connect ping stream - var pongStream = new PongStream({ objectMode: true }) + const pongStream = new PongStream({ objectMode: true }) pongStream.pipe(mx.createStream('pingpong')).pipe(pongStream) - // ignore unused channels (handled by background) + // connect phishing warning stream + const phishingStream = mx.createStream('phishing') + phishingStream.once('data', redirectToPhishingWarning) + + // ignore unused channels (handled by background, inpage) mx.ignoreStream('provider') mx.ignoreStream('publicConfig') } @@ -88,3 +93,8 @@ function suffixCheck () { } return true } + +function redirectToPhishingWarning () { + console.log('MetaMask - redirecting to phishing warning') + window.location.href = 'https://metamask.io/phishing.html' +} diff --git a/app/scripts/controllers/blacklist.js b/app/scripts/controllers/blacklist.js new file mode 100644 index 000000000..7e01fa386 --- /dev/null +++ b/app/scripts/controllers/blacklist.js @@ -0,0 +1,58 @@ +const ObservableStore = require('obs-store') +const extend = require('xtend') +const PhishingDetector = require('eth-phishing-detect/src/detector') + +// compute phishing lists +const PHISHING_DETECTION_CONFIG = require('eth-phishing-detect/src/config.json') +// every ten minutes +const POLLING_INTERVAL = 10 * 60 * 1000 + +class BlacklistController { + + constructor (opts = {}) { + const initState = extend({ + phishing: PHISHING_DETECTION_CONFIG, + }, opts.initState) + this.store = new ObservableStore(initState) + // phishing detector + this._phishingDetector = null + this._setupPhishingDetector(initState.phishing) + // polling references + this._phishingUpdateIntervalRef = null + } + + // + // PUBLIC METHODS + // + + checkForPhishing (hostname) { + if (!hostname) return false + const { result } = this._phishingDetector.check(hostname) + return result + } + + async updatePhishingList () { + const response = await fetch('https://api.infura.io/v2/blacklist') + const phishing = await response.json() + this.store.updateState({ phishing }) + this._setupPhishingDetector(phishing) + return phishing + } + + scheduleUpdates () { + if (this._phishingUpdateIntervalRef) return + this._phishingUpdateIntervalRef = setInterval(() => { + this.updatePhishingList() + }, POLLING_INTERVAL) + } + + // + // PRIVATE METHODS + // + + _setupPhishingDetector (config) { + this._phishingDetector = new PhishingDetector(config) + } +} + +module.exports = BlacklistController diff --git a/app/scripts/controllers/infura.js b/app/scripts/controllers/infura.js index b34b0bc03..10adb1004 100644 --- a/app/scripts/controllers/infura.js +++ b/app/scripts/controllers/infura.js @@ -2,7 +2,7 @@ const ObservableStore = require('obs-store') const extend = require('xtend') // every ten minutes -const POLLING_INTERVAL = 300000 +const POLLING_INTERVAL = 10 * 60 * 1000 class InfuraController { diff --git a/app/scripts/controllers/network.js b/app/scripts/controllers/network.js index c07f13b8d..0a3e5e26b 100644 --- a/app/scripts/controllers/network.js +++ b/app/scripts/controllers/network.js @@ -28,9 +28,9 @@ module.exports = class NetworkController extends EventEmitter { this._provider = provider } - initializeProvider (opts) { + initializeProvider (opts, providerContructor = MetaMaskProvider) { this.providerInit = opts - this._provider = MetaMaskProvider(opts) + this._provider = providerContructor(opts) this._proxy = new Proxy(this._provider, { get: (obj, name) => { if (name === 'on') return this._on.bind(this) @@ -38,6 +38,7 @@ module.exports = class NetworkController extends EventEmitter { }, set: (obj, name, value) => { this._provider[name] = value + return value }, }) this.provider.on('block', this._logBlock.bind(this)) diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index 5f3d84ebe..308d43cb0 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -1,9 +1,9 @@ const EventEmitter = require('events') -const async = require('async') const extend = require('xtend') +const clone = require('clone') const ObservableStore = require('obs-store') const ethUtil = require('ethereumjs-util') -const pify = require('pify') +const EthQuery = require('ethjs-query') const TxProviderUtil = require('../lib/tx-utils') const createId = require('../lib/random-id') const NonceTracker = require('../lib/nonce-tracker') @@ -22,7 +22,6 @@ module.exports = class TransactionController extends EventEmitter { this.blockTracker = opts.blockTracker this.nonceTracker = new NonceTracker({ provider: this.provider, - blockTracker: this.provider._blockTracker, getPendingTransactions: (address) => { return this.getFilteredTxList({ from: address, @@ -31,7 +30,7 @@ module.exports = class TransactionController extends EventEmitter { }) }, }) - this.query = opts.ethQuery + this.query = new EthQuery(this.provider) this.txProviderUtils = new TxProviderUtil(this.query) this.blockTracker.on('rawBlock', this.checkForTxInBlock.bind(this)) // this is a little messy but until ethstore has been either @@ -60,13 +59,6 @@ module.exports = class TransactionController extends EventEmitter { return this.preferencesStore.getState().selectedAddress } - // Returns the tx list - getTxList () { - const network = this.getNetwork() - const fullTxList = this.getFullTxList() - return fullTxList.filter(txMeta => txMeta.metamaskNetworkId === network) - } - // Returns the number of txs for the current network. getTxCount () { return this.getTxList().length @@ -77,6 +69,56 @@ module.exports = class TransactionController extends EventEmitter { return this.store.getState().transactions } + getUnapprovedTxCount () { + return Object.keys(this.getUnapprovedTxList()).length + } + + getPendingTxCount () { + return this.getTxsByMetaData('status', 'signed').length + } + + // Returns the tx list + getTxList () { + const network = this.getNetwork() + const fullTxList = this.getFullTxList() + return this.getTxsByMetaData('metamaskNetworkId', network, fullTxList) + } + + // gets tx by Id and returns it + getTx (txId) { + const txList = this.getTxList() + const txMeta = txList.find(txData => txData.id === txId) + return txMeta + } + getUnapprovedTxList () { + const txList = this.getTxList() + return txList.filter((txMeta) => txMeta.status === 'unapproved') + .reduce((result, tx) => { + result[tx.id] = tx + return result + }, {}) + } + + updateTx (txMeta) { + // create txMeta snapshot for history + const txMetaForHistory = clone(txMeta) + // dont include previous history in this snapshot + delete txMetaForHistory.history + // add snapshot to tx history + if (!txMeta.history) txMeta.history = [] + txMeta.history.push(txMetaForHistory) + + const txId = txMeta.id + const txList = this.getFullTxList() + const index = txList.findIndex(txData => txData.id === txId) + if (!txMeta.history) txMeta.history = [] + txMeta.history.push(txMetaForHistory) + + txList[index] = txMeta + this._saveTxList(txList) + this.emit('update') + } + // Adds a tx to the txlist addTx (txMeta) { const txCount = this.getTxCount() @@ -90,7 +132,7 @@ module.exports = class TransactionController extends EventEmitter { // or rejected tx's. // not tx's that are pending or unapproved if (txCount > txHistoryLimit - 1) { - var index = fullTxList.findIndex((metaTx) => ((metaTx.status === 'confirmed' || metaTx.status === 'rejected') && network === txMeta.metamaskNetworkId)) + const index = fullTxList.findIndex((metaTx) => ((metaTx.status === 'confirmed' || metaTx.status === 'rejected') && network === txMeta.metamaskNetworkId)) fullTxList.splice(index, 1) } fullTxList.push(txMeta) @@ -108,79 +150,59 @@ module.exports = class TransactionController extends EventEmitter { this.emit(`${txMeta.id}:unapproved`, txMeta) } - // gets tx by Id and returns it - getTx (txId, cb) { - var txList = this.getTxList() - var txMeta = txList.find(txData => txData.id === txId) - return cb ? cb(txMeta) : txMeta - } - - // - updateTx (txMeta) { - var txId = txMeta.id - var txList = this.getFullTxList() - var index = txList.findIndex(txData => txData.id === txId) - txList[index] = txMeta - this._saveTxList(txList) - this.emit('update') - } - - get unapprovedTxCount () { - return Object.keys(this.getUnapprovedTxList()).length - } - - get pendingTxCount () { - return this.getTxsByMetaData('status', 'signed').length - } - - addUnapprovedTransaction (txParams, done) { - let txMeta - async.waterfall([ - // validate - (cb) => this.txProviderUtils.validateTxParams(txParams, cb), - // construct txMeta - (cb) => { - txMeta = { - id: createId(), - time: (new Date()).getTime(), - status: 'unapproved', - metamaskNetworkId: this.getNetwork(), - txParams: txParams, + async newUnapprovedTransaction (txParams) { + log.debug(`MetaMaskController newUnapprovedTransaction ${JSON.stringify(txParams)}`) + const txMeta = await this.addUnapprovedTransaction(txParams) + this.emit('newUnaprovedTx', txMeta) + // listen for tx completion (success, fail) + return new Promise((resolve, reject) => { + this.once(`${txMeta.id}:finished`, (completedTx) => { + switch (completedTx.status) { + case 'submitted': + return resolve(completedTx.hash) + case 'rejected': + return reject(new Error('MetaMask Tx Signature: User denied transaction signature.')) + default: + return reject(new Error(`MetaMask Tx Signature: Unknown problem: ${JSON.stringify(completedTx.txParams)}`)) } - cb() - }, - // add default tx params - (cb) => this.addTxDefaults(txMeta, cb), - // save txMeta - (cb) => { - this.addTx(txMeta) - cb(null, txMeta) - }, - ], done) + }) + }) } - addTxDefaults (txMeta, cb) { + async addUnapprovedTransaction (txParams) { + // validate + await this.txProviderUtils.validateTxParams(txParams) + // construct txMeta + const txMeta = { + id: createId(), + time: (new Date()).getTime(), + status: 'unapproved', + metamaskNetworkId: this.getNetwork(), + txParams: txParams, + history: [], + } + // add default tx params + await this.addTxDefaults(txMeta) + // save txMeta + this.addTx(txMeta) + return txMeta + } + + async addTxDefaults (txMeta) { const txParams = txMeta.txParams // ensure value txParams.value = txParams.value || '0x0' if (!txParams.gasPrice) { - this.query.gasPrice((err, gasPrice) => { - if (err) return cb(err) - // set gasPrice - txParams.gasPrice = gasPrice - }) + const gasPrice = await this.query.gasPrice() + txParams.gasPrice = gasPrice } // set gasLimit - this.txProviderUtils.analyzeGasUsage(txMeta, cb) + return await this.txProviderUtils.analyzeGasUsage(txMeta) } - getUnapprovedTxList () { - var txList = this.getTxList() - return txList.filter((txMeta) => txMeta.status === 'unapproved') - .reduce((result, tx) => { - result[tx.id] = tx - return result - }, {}) + async updateAndApproveTransaction (txMeta) { + this.updateTx(txMeta) + await this.approveTransaction(txMeta.id) } async approveTransaction (txId) { @@ -191,8 +213,12 @@ module.exports = class TransactionController extends EventEmitter { // get next nonce const txMeta = this.getTx(txId) const fromAddress = txMeta.txParams.from + // wait for a nonce nonceLock = await this.nonceTracker.getNonceLock(fromAddress) + // add nonce to txParams txMeta.txParams.nonce = nonceLock.nextNonce + // add nonce debugging information to txMeta + txMeta.nonceDetails = nonceLock.nonceDetails this.updateTx(txMeta) // sign transaction const rawTx = await this.signTransaction(txId) @@ -201,6 +227,7 @@ module.exports = class TransactionController extends EventEmitter { nonceLock.releaseLock() } catch (err) { this.setTxStatusFailed(txId, { + stack: err.stack || err.message, errCode: err.errCode || err, message: err.message || 'Transaction failed during approval', }) @@ -211,16 +238,33 @@ module.exports = class TransactionController extends EventEmitter { } } - cancelTransaction (txId, cb = warn) { - this.setTxStatusRejected(txId) - cb() + async signTransaction (txId) { + const txMeta = this.getTx(txId) + const txParams = txMeta.txParams + const fromAddress = txParams.from + // add network/chain id + txParams.chainId = this.getChainId() + const ethTx = this.txProviderUtils.buildEthTxFromParams(txParams) + await this.signEthTx(ethTx, fromAddress) + this.setTxStatusSigned(txMeta.id) + const rawTx = ethUtil.bufferToHex(ethTx.serialize()) + return rawTx } - async updateAndApproveTransaction (txMeta) { + async publishTransaction (txId, rawTx) { + const txMeta = this.getTx(txId) + txMeta.rawTx = rawTx this.updateTx(txMeta) - await this.approveTransaction(txMeta.id) + const txHash = await this.txProviderUtils.publishTransaction(rawTx) + this.setTxHash(txId, txHash) + this.setTxStatusSubmitted(txId) } + async cancelTransaction (txId) { + this.setTxStatusRejected(txId) + } + + getChainId () { const networkState = this.networkStore.getState() const getChainId = parseInt(networkState) @@ -231,30 +275,6 @@ module.exports = class TransactionController extends EventEmitter { } } - async signTransaction (txId) { - const txMeta = this.getTx(txId) - const txParams = txMeta.txParams - const fromAddress = txParams.from - // add network/chain id - txParams.chainId = this.getChainId() - const ethTx = this.txProviderUtils.buildEthTxFromParams(txParams) - const rawTx = await this.signEthTx(ethTx, fromAddress).then(() => { - this.setTxStatusSigned(txMeta.id) - return ethUtil.bufferToHex(ethTx.serialize()) - }) - return rawTx - } - - async publishTransaction (txId, rawTx) { - const txMeta = this.getTx(txId) - txMeta.rawTx = rawTx - this.updateTx(txMeta) - await this.txProviderUtils.publishTransaction(rawTx).then((txHash) => { - this.setTxHash(txId, txHash) - this.setTxStatusSubmitted(txId) - }) - } - // receives a txHash records the tx as signed setTxHash (txId, txHash) { // Add the tx hash to the persisted meta-tx object @@ -265,7 +285,7 @@ module.exports = class TransactionController extends EventEmitter { /* Takes an object of fields to search for eg: - var thingsToLookFor = { + let thingsToLookFor = { to: '0x0..', from: '0x0..', status: 'signed', @@ -288,7 +308,7 @@ module.exports = class TransactionController extends EventEmitter { and that have been 'confirmed' */ getFilteredTxList (opts) { - var filteredTxList + let filteredTxList Object.keys(opts).forEach((key) => { filteredTxList = this.getTxsByMetaData(key, opts[key], filteredTxList) }) @@ -349,7 +369,7 @@ module.exports = class TransactionController extends EventEmitter { // merges txParams obj onto txData.txParams // use extend to ensure that all fields are filled updateTxParams (txId, txParams) { - var txMeta = this.getTx(txId) + const txMeta = this.getTx(txId) txMeta.txParams = extend(txMeta.txParams, txParams) this.updateTx(txMeta) } @@ -357,20 +377,19 @@ module.exports = class TransactionController extends EventEmitter { // checks if a signed tx is in a block and // if included sets the tx status as 'confirmed' checkForTxInBlock (block) { - var signedTxList = this.getFilteredTxList({status: 'submitted'}) + const signedTxList = this.getFilteredTxList({status: 'submitted'}) if (!signedTxList.length) return signedTxList.forEach((txMeta) => { - var txHash = txMeta.hash - var txId = txMeta.id + const txHash = txMeta.hash + const txId = txMeta.id if (!txHash) { - const errReason = { - errCode: 'No hash was provided', - message: 'We had an error while submitting this transaction, please try again.', - } - return this.setTxStatusFailed(txId, errReason) + const noTxHashErr = new Error('We had an error while submitting this transaction, please try again.') + noTxHashErr.name = 'NoTxHashError' + this.setTxStatusFailed(txId, noTxHashErr) } + block.transactions.forEach((tx) => { if (tx.hash === txHash) this.setTxStatusConfirmed(txId) }) @@ -388,44 +407,6 @@ module.exports = class TransactionController extends EventEmitter { if (diff > 1) this._checkPendingTxs() } - // PRIVATE METHODS - - // Should find the tx in the tx list and - // update it. - // should set the status in txData - // - `'unapproved'` the user has not responded - // - `'rejected'` the user has responded no! - // - `'approved'` the user has approved the tx - // - `'signed'` the tx is signed - // - `'submitted'` the tx is sent to a server - // - `'confirmed'` the tx has been included in a block. - // - `'failed'` the tx failed for some reason, included on tx data. - _setTxStatus (txId, status) { - var txMeta = this.getTx(txId) - txMeta.status = status - this.emit(`${txMeta.id}:${status}`, txId) - if (status === 'submitted' || status === 'rejected') { - this.emit(`${txMeta.id}:finished`, txMeta) - } - this.updateTx(txMeta) - this.emit('updateBadge') - } - - // Saves the new/updated txList. - // Function is intended only for internal use - _saveTxList (transactions) { - this.store.updateState({ transactions }) - } - - _updateMemstore () { - const unapprovedTxs = this.getUnapprovedTxList() - const selectedAddressTxList = this.getFilteredTxList({ - from: this.getSelectedAddress(), - metamaskNetworkId: this.getNetwork(), - }) - this.memStore.updateState({ unapprovedTxs, selectedAddressTxList }) - } - resubmitPendingTxs () { const pending = this.getTxsByMetaData('status', 'submitted') // only try resubmitting if their are transactions to resubmit @@ -452,13 +433,57 @@ module.exports = class TransactionController extends EventEmitter { if (isKnownTx) return // encountered real error - transition to error state this.setTxStatusFailed(txMeta.id, { + stack: err.stack || err.message, errCode: err.errCode || err, message: err.message, }) })) } - async _resubmitTx (txMeta, cb) { + +/* _____________________________________ +| | +| PRIVATE METHODS | +|______________________________________*/ + + + // Should find the tx in the tx list and + // update it. + // should set the status in txData + // - `'unapproved'` the user has not responded + // - `'rejected'` the user has responded no! + // - `'approved'` the user has approved the tx + // - `'signed'` the tx is signed + // - `'submitted'` the tx is sent to a server + // - `'confirmed'` the tx has been included in a block. + // - `'failed'` the tx failed for some reason, included on tx data. + _setTxStatus (txId, status) { + const txMeta = this.getTx(txId) + txMeta.status = status + this.emit(`${txMeta.id}:${status}`, txId) + if (status === 'submitted' || status === 'rejected') { + this.emit(`${txMeta.id}:finished`, txMeta) + } + this.updateTx(txMeta) + this.emit('updateBadge') + } + + // Saves the new/updated txList. + // Function is intended only for internal use + _saveTxList (transactions) { + this.store.updateState({ transactions }) + } + + _updateMemstore () { + const unapprovedTxs = this.getUnapprovedTxList() + const selectedAddressTxList = this.getFilteredTxList({ + from: this.getSelectedAddress(), + metamaskNetworkId: this.getNetwork(), + }) + this.memStore.updateState({ unapprovedTxs, selectedAddressTxList }) + } + + async _resubmitTx (txMeta) { const address = txMeta.txParams.from const balance = this.ethStore.getState().accounts[address].balance if (!('retryCount' in txMeta)) txMeta.retryCount = 0 @@ -466,18 +491,21 @@ module.exports = class TransactionController extends EventEmitter { // if the value of the transaction is greater then the balance, fail. if (!this.txProviderUtils.sufficientBalance(txMeta.txParams, balance)) { const message = 'Insufficient balance.' - this.setTxStatusFailed(txMeta.id, { message }) - cb() - return log.error(message) + this.setTxStatusFailed(txMeta.id, { + stack: '_resubmitTx: custom tx-controller error', + message, + }) + log.error(message) + return } // Only auto-submit already-signed txs: - if (!('rawTx' in txMeta)) return cb() + if (!('rawTx' in txMeta)) return // Increment a try counter. txMeta.retryCount++ const rawTx = txMeta.rawTx - return await this.txProviderUtils.publishTransaction(rawTx, cb) + return await this.txProviderUtils.publishTransaction(rawTx) } // checks the network for signed txs and @@ -501,17 +529,14 @@ module.exports = class TransactionController extends EventEmitter { // extra check in case there was an uncaught error during the // signature and submission process if (!txHash) { - const errReason = { - errCode: 'No hash was provided', - message: 'We had an error while submitting this transaction, please try again.', - } - this.setTxStatusFailed(txId, errReason) - return + const noTxHashErr = new Error('We had an error while submitting this transaction, please try again.') + noTxHashErr.name = 'NoTxHashError' + this.setTxStatusFailed(txId, noTxHashErr) } // get latest transaction status let txParams try { - txParams = await pify((cb) => this.query.getTransactionByHash(txHash, cb))() + txParams = await this.query.getTransactionByHash(txHash) if (!txParams) return if (txParams.blockNumber) { this.setTxStatusConfirmed(txId) @@ -524,12 +549,8 @@ module.exports = class TransactionController extends EventEmitter { message: 'There was a problem loading this transaction.', } this.updateTx(txMeta) - log.error(err) + throw err } } } - -} - - -const warn = () => log.warn('warn was used no cb provided') +} \ No newline at end of file diff --git a/app/scripts/inpage.js b/app/scripts/inpage.js index ec764535e..9e98c044b 100644 --- a/app/scripts/inpage.js +++ b/app/scripts/inpage.js @@ -65,3 +65,4 @@ function restoreContextAfterImports () { console.warn('MetaMask - global.define could not be overwritten.') } } + diff --git a/app/scripts/lib/inpage-provider.js b/app/scripts/lib/inpage-provider.js index 8b8623974..fd032a673 100644 --- a/app/scripts/lib/inpage-provider.js +++ b/app/scripts/lib/inpage-provider.js @@ -26,6 +26,9 @@ function MetamaskInpageProvider (connectionStream) { (err) => logStreamDisconnectWarning('MetaMask PublicConfigStore', err) ) + // ignore phishing warning message (handled elsewhere) + multiStream.ignoreStream('phishing') + // connect to async provider const asyncProvider = self.asyncProvider = new StreamProvider() pipe( diff --git a/app/scripts/lib/nonce-tracker.js b/app/scripts/lib/nonce-tracker.js index b76dac4e8..8328e81ec 100644 --- a/app/scripts/lib/nonce-tracker.js +++ b/app/scripts/lib/nonce-tracker.js @@ -4,8 +4,8 @@ const Mutex = require('await-semaphore').Mutex class NonceTracker { - constructor ({ blockTracker, provider, getPendingTransactions }) { - this.blockTracker = blockTracker + constructor ({ provider, getPendingTransactions }) { + this.provider = provider this.ethQuery = new EthQuery(provider) this.getPendingTransactions = getPendingTransactions this.lockMap = {} @@ -31,21 +31,25 @@ class NonceTracker { const currentBlock = await this._getCurrentBlock() const pendingTransactions = this.getPendingTransactions(address) const pendingCount = pendingTransactions.length - assert(Number.isInteger(pendingCount), 'nonce-tracker - pendingCount is an integer') + assert(Number.isInteger(pendingCount), `nonce-tracker - pendingCount is not an integer - got: (${typeof pendingCount}) "${pendingCount}"`) const baseCountHex = await this._getTxCount(address, currentBlock) const baseCount = parseInt(baseCountHex, 16) - assert(Number.isInteger(baseCount), 'nonce-tracker - baseCount is an integer') + assert(Number.isInteger(baseCount), `nonce-tracker - baseCount is not an integer - got: (${typeof baseCount}) "${baseCount}"`) const nextNonce = baseCount + pendingCount - assert(Number.isInteger(nextNonce), 'nonce-tracker - nextNonce is an integer') - // return next nonce and release cb - return { nextNonce, releaseLock } + assert(Number.isInteger(nextNonce), `nonce-tracker - nextNonce is not an integer - got: (${typeof nextNonce}) "${nextNonce}"`) + // collect the numbers used to calculate the nonce for debugging + const blockNumber = currentBlock.number + const nonceDetails = { blockNumber, baseCount, baseCountHex, pendingCount } + // return nonce and release cb + return { nextNonce, nonceDetails, releaseLock } } async _getCurrentBlock () { - const currentBlock = this.blockTracker.getCurrentBlock() + const blockTracker = this._getBlockTracker() + const currentBlock = blockTracker.getCurrentBlock() if (currentBlock) return currentBlock return await Promise((reject, resolve) => { - this.blockTracker.once('latest', resolve) + blockTracker.once('latest', resolve) }) } @@ -79,6 +83,12 @@ class NonceTracker { return mutex } + // this is a hotfix for the fact that the blockTracker will + // change when the network changes + _getBlockTracker () { + return this.provider._blockTracker + } + } module.exports = NonceTracker diff --git a/app/scripts/lib/obj-multiplex.js b/app/scripts/lib/obj-multiplex.js index bd114c394..0034febe0 100644 --- a/app/scripts/lib/obj-multiplex.js +++ b/app/scripts/lib/obj-multiplex.js @@ -5,12 +5,16 @@ module.exports = ObjectMultiplex function ObjectMultiplex (opts) { opts = opts || {} // create multiplexer - var mx = through.obj(function (chunk, enc, cb) { - var name = chunk.name - var data = chunk.data - var substream = mx.streams[name] + 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(`orphaned data for stream "${name}"`) + console.warn(`ObjectMultiplex - orphaned data for stream "${name}"`) } else { if (substream.push) substream.push(data) } @@ -19,7 +23,7 @@ function ObjectMultiplex (opts) { mx.streams = {} // create substreams mx.createStream = function (name) { - var substream = mx.streams[name] = through.obj(function (chunk, enc, cb) { + const substream = mx.streams[name] = through.obj(function (chunk, enc, cb) { mx.push({ name: name, data: chunk, diff --git a/app/scripts/lib/tx-utils.js b/app/scripts/lib/tx-utils.js index 8f6943937..3687a9652 100644 --- a/app/scripts/lib/tx-utils.js +++ b/app/scripts/lib/tx-utils.js @@ -1,4 +1,3 @@ -const async = require('async') const ethUtil = require('ethereumjs-util') const Transaction = require('ethereumjs-tx') const normalize = require('eth-sig-util').normalize @@ -10,24 +9,19 @@ its passed ethquery and used to do things like calculate gas of a tx. */ -module.exports = class txProviderUtils { - +module.exports = class txProvideUtils { constructor (ethQuery) { this.query = ethQuery } - analyzeGasUsage (txMeta, cb) { - var self = this - this.query.getBlockByNumber('latest', true, (err, block) => { - if (err) return cb(err) - async.waterfall([ - self.estimateTxGas.bind(self, txMeta, block.gasLimit), - self.setTxGas.bind(self, txMeta, block.gasLimit), - ], cb) - }) + async analyzeGasUsage (txMeta) { + const block = await this.query.getBlockByNumber('latest', true) + const estimatedGasHex = await this.estimateTxGas(txMeta, block.gasLimit) + this.setTxGas(txMeta, block.gasLimit, estimatedGasHex) + return txMeta } - estimateTxGas (txMeta, blockGasLimitHex, cb) { + async estimateTxGas (txMeta, blockGasLimitHex) { const txParams = txMeta.txParams // check if gasLimit is already specified txMeta.gasLimitSpecified = Boolean(txParams.gas) @@ -38,10 +32,10 @@ module.exports = class txProviderUtils { txParams.gas = bnToHex(saferGasLimitBN) } // run tx, see if it will OOG - this.query.estimateGas(txParams, cb) + return this.query.estimateGas(txParams) } - setTxGas (txMeta, blockGasLimitHex, estimatedGasHex, cb) { + setTxGas (txMeta, blockGasLimitHex, estimatedGasHex) { txMeta.estimatedGas = estimatedGasHex const txParams = txMeta.txParams @@ -49,14 +43,12 @@ module.exports = class txProviderUtils { // use original specified amount if (txMeta.gasLimitSpecified) { txMeta.estimatedGas = txParams.gas - cb() return } // if gasLimit not originally specified, // try adding an additional gas buffer to our estimation for safety const recommendedGasHex = this.addGasBuffer(txMeta.estimatedGas, blockGasLimitHex) txParams.gas = recommendedGasHex - cb() return } @@ -74,22 +66,6 @@ module.exports = class txProviderUtils { return bnToHex(upperGasLimitBn) } - fillInTxParams (txParams, cb) { - const fromAddress = txParams.from - const reqs = {} - - if (isUndef(txParams.gas)) reqs.gas = (cb) => this.query.estimateGas(txParams, cb) - if (isUndef(txParams.gasPrice)) reqs.gasPrice = (cb) => this.query.gasPrice(cb) - if (isUndef(txParams.nonce)) reqs.nonce = (cb) => this.query.getTransactionCount(fromAddress, 'pending', cb) - - async.parallel(reqs, function (err, result) { - if (err) return cb(err) - // write results to txParams obj - Object.assign(txParams, result) - cb() - }) - } - // builds ethTx from txParams object buildEthTxFromParams (txParams) { // normalize values @@ -106,20 +82,13 @@ module.exports = class txProviderUtils { return ethTx } - publishTransaction (rawTx) { - return new Promise((resolve, reject) => { - this.query.sendRawTransaction(rawTx, (err, ress) => { - if (err) reject(err) - else resolve(ress) - }) - }) + async publishTransaction (rawTx) { + return await this.query.sendRawTransaction(rawTx) } - validateTxParams (txParams, cb) { + async validateTxParams (txParams) { if (('value' in txParams) && txParams.value.indexOf('-') === 0) { - cb(new Error(`Invalid transaction value of ${txParams.value} not a positive number.`)) - } else { - cb() + throw new Error(`Invalid transaction value of ${txParams.value} not a positive number.`) } } @@ -137,10 +106,6 @@ module.exports = class txProviderUtils { // util -function isUndef (value) { - return value === undefined -} - function bnToHex (inputBn) { return ethUtil.addHexPrefix(inputBn.toString(16)) } diff --git a/app/scripts/lib/util.js b/app/scripts/lib/util.js new file mode 100644 index 000000000..bddd60ee8 --- /dev/null +++ b/app/scripts/lib/util.js @@ -0,0 +1,8 @@ +module.exports = { + getStack, +} + +function getStack () { + const stack = new Error('Stack trace generator - not an error').stack + return stack +} diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 11dcde2c1..a007d6fc5 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -16,6 +16,7 @@ const NoticeController = require('./notice-controller') const ShapeShiftController = require('./controllers/shapeshift') const AddressBookController = require('./controllers/address-book') const InfuraController = require('./controllers/infura') +const BlacklistController = require('./controllers/blacklist') const MessageManager = require('./lib/message-manager') const PersonalMessageManager = require('./lib/personal-message-manager') const TransactionController = require('./controllers/transactions') @@ -69,6 +70,10 @@ module.exports = class MetamaskController extends EventEmitter { }) this.infuraController.scheduleInfuraNetworkCheck() + this.blacklistController = new BlacklistController({ + initState: initState.BlacklistController, + }) + this.blacklistController.scheduleUpdates() // rpc provider this.provider = this.initializeProvider() @@ -108,6 +113,7 @@ module.exports = class MetamaskController extends EventEmitter { ethQuery: this.ethQuery, ethStore: this.ethStore, }) + this.txController.on('newUnaprovedTx', opts.showUnapprovedTx.bind(opts)) // notices this.noticeController = new NoticeController({ @@ -151,6 +157,9 @@ module.exports = class MetamaskController extends EventEmitter { this.networkController.store.subscribe((state) => { this.store.updateState({ NetworkController: state }) }) + this.blacklistController.store.subscribe((state) => { + this.store.updateState({ BlacklistController: state }) + }) this.infuraController.store.subscribe((state) => { this.store.updateState({ InfuraController: state }) }) @@ -195,7 +204,7 @@ module.exports = class MetamaskController extends EventEmitter { cb(null, result) }, // tx signing - processTransaction: (txParams, cb) => this.newUnapprovedTransaction(txParams, cb), + processTransaction: nodeify(async (txParams) => await this.txController.newUnapprovedTransaction(txParams), this), // old style msg signing processMessage: this.newUnsignedMessage.bind(this), @@ -308,7 +317,7 @@ module.exports = class MetamaskController extends EventEmitter { exportAccount: nodeify(keyringController.exportAccount, keyringController), // txController - cancelTransaction: txController.cancelTransaction.bind(txController), + cancelTransaction: nodeify(txController.cancelTransaction, txController), updateAndApproveTransaction: nodeify(txController.updateAndApproveTransaction, txController), // messageManager @@ -326,8 +335,15 @@ 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) + this.sendPhishingWarning(connectionStream, originDomain) + return + } + // setup multiplexing - var mx = setupMultiplex(connectionStream) + const mx = setupMultiplex(connectionStream) // connect features this.setupProviderConnection(mx.createStream('provider'), originDomain) this.setupPublicConfig(mx.createStream('publicConfig')) @@ -335,12 +351,18 @@ module.exports = class MetamaskController extends EventEmitter { setupTrustedCommunication (connectionStream, originDomain) { // setup multiplexing - var mx = setupMultiplex(connectionStream) + const mx = setupMultiplex(connectionStream) // connect features this.setupControllerConnection(mx.createStream('controller')) this.setupProviderConnection(mx.createStream('provider'), originDomain) } + sendPhishingWarning (connectionStream, hostname) { + const mx = setupMultiplex(connectionStream) + const phishingStream = mx.createStream('phishing') + phishingStream.write({ hostname }) + } + setupControllerConnection (outStream) { const api = this.getApi() const dnode = Dnode(api) @@ -440,27 +462,6 @@ module.exports = class MetamaskController extends EventEmitter { // Identity Management // - newUnapprovedTransaction (txParams, cb) { - log.debug(`MetaMaskController newUnapprovedTransaction ${JSON.stringify(txParams)}`) - const self = this - self.txController.addUnapprovedTransaction(txParams, (err, txMeta) => { - if (err) return cb(err) - self.sendUpdate() - self.opts.showUnapprovedTx(txMeta) - // listen for tx completion (success, fail) - self.txController.once(`${txMeta.id}:finished`, (completedTx) => { - switch (completedTx.status) { - case 'submitted': - return cb(null, completedTx.hash) - case 'rejected': - return cb(new Error('MetaMask Tx Signature: User denied transaction signature.')) - default: - return cb(new Error(`MetaMask Tx Signature: Unknown problem: ${JSON.stringify(completedTx.txParams)}`)) - } - }) - }) - } - newUnsignedMessage (msgParams, cb) { const msgId = this.messageManager.addUnapprovedMessage(msgParams) this.sendUpdate() @@ -646,6 +647,4 @@ module.exports = class MetamaskController extends EventEmitter { return Promise.resolve(rpcTarget) }) } - - -} +} \ No newline at end of file diff --git a/circle.yml b/circle.yml index 66eed17d7..2ea60bb9d 100644 --- a/circle.yml +++ b/circle.yml @@ -5,3 +5,6 @@ dependencies: pre: - "npm i -g testem" - "npm i -g mocha" +test: + override: + - "npm run ci" \ No newline at end of file diff --git a/gulpfile.js b/gulpfile.js index 53de7a7d9..cc723704a 100644 --- a/gulpfile.js +++ b/gulpfile.js @@ -172,7 +172,6 @@ gulp.task('default', ['lint'], function () { const jsFiles = [ 'inpage', 'contentscript', - 'blacklister', 'background', 'popup', ] diff --git a/package.json b/package.json index 2e66080d6..8cc0e6936 100644 --- a/package.json +++ b/package.json @@ -7,12 +7,14 @@ "start": "npm run dev", "dev": "gulp dev --debug", "disc": "gulp disc --debug", - "clear": "rm -rf node_modules/eth-contract-metadata && rm -rf node_modules/etheraddresslookup", + "clear": "rm -rf node_modules/eth-contract-metadata && rm -rf node_modules/eth-phishing-detect", "dist": "npm run clear && npm install && gulp dist", "test": "npm run lint && npm run test-unit && npm run test-integration", "test-unit": "METAMASK_ENV=test mocha --require test/helper.js --recursive \"test/unit/**/*.js\"", "single-test": "METAMASK_ENV=test mocha --require test/helper.js", "test-integration": "npm run buildMock && npm run buildCiUnits && testem ci -P 2", + "test-coverage": "nyc npm run test-unit && nyc report --reporter=text-lcov | coveralls", + "ci": "npm run lint && npm run test-coverage && npm run test-integration", "lint": "gulp lint", "buildCiUnits": "node test/integration/index.js", "watch": "mocha watch --recursive \"test/unit/**/*.js\"", @@ -66,19 +68,21 @@ "eth-bin-to-ops": "^1.0.1", "eth-contract-metadata": "^1.1.4", "eth-hd-keyring": "^1.1.1", + "eth-phishing-detect": "^1.0.2", "eth-query": "^2.1.2", - "eth-sig-util": "^1.1.1", + "eth-sig-util": "^1.2.2", "eth-simple-keyring": "^1.1.1", "eth-token-tracker": "^1.1.2", - "etheraddresslookup": "github:409H/EtherAddressLookup", "ethereumjs-tx": "^1.3.0", "ethereumjs-util": "ethereumjs/ethereumjs-util#ac5d0908536b447083ea422b435da27f26615de9", "ethereumjs-wallet": "^0.6.0", "ethjs-ens": "^2.0.0", + "ethjs-query": "^0.2.6", "express": "^4.14.0", "extension-link-enabler": "^1.0.0", "extensionizer": "^1.0.0", "gulp-eslint": "^4.0.0", + "fast-levenshtein": "^2.0.6", "hat": "0.0.3", "idb-global": "^2.1.0", "identicon.js": "^2.3.1", @@ -122,12 +126,11 @@ "semaphore": "^1.0.5", "sw-stream": "^2.0.0", "textarea-caret": "^3.0.1", - "three.js": "^0.77.1", - "through2": "^2.0.1", "valid-url": "^1.0.9", "vreme": "^3.0.2", "web3": "0.20.1", - "web3-provider-engine": "^13.2.8", + "through2": "^2.0.3", + "web3-provider-engine": "^13.2.9", "web3-stream-provider": "^3.0.1", "xtend": "^4.0.1" }, @@ -144,6 +147,7 @@ "brfs": "^1.4.3", "browserify": "^14.4.0", "chai": "^4.1.0", + "coveralls": "^2.13.1", "deep-freeze-strict": "^1.1.1", "del": "^3.0.0", "envify": "^4.0.0", @@ -170,6 +174,7 @@ "mocha-jsdom": "^1.1.0", "mocha-sinon": "^2.0.0", "nock": "^9.0.14", + "nyc": "^11.0.3", "open": "0.0.5", "prompt": "^1.0.0", "qs": "^6.2.0", diff --git a/test/unit/blacklist-controller-test.js b/test/unit/blacklist-controller-test.js new file mode 100644 index 000000000..a9260466f --- /dev/null +++ b/test/unit/blacklist-controller-test.js @@ -0,0 +1,41 @@ +const assert = require('assert') +const BlacklistController = require('../../app/scripts/controllers/blacklist') + +describe('blacklist controller', function () { + let blacklistController + + before(() => { + blacklistController = new BlacklistController() + }) + + describe('checkForPhishing', function () { + it('should not flag whitelisted values', function () { + const result = blacklistController.checkForPhishing('www.metamask.io') + assert.equal(result, false) + }) + it('should flag explicit values', function () { + const result = blacklistController.checkForPhishing('metamask.com') + assert.equal(result, true) + }) + it('should flag levenshtein values', function () { + const result = blacklistController.checkForPhishing('metmask.io') + assert.equal(result, true) + }) + it('should not flag not-even-close values', function () { + const result = blacklistController.checkForPhishing('example.com') + assert.equal(result, false) + }) + it('should not flag the ropsten faucet domains', function () { + const result = blacklistController.checkForPhishing('faucet.metamask.io') + assert.equal(result, false) + }) + it('should not flag the mascara domain', function () { + const result = blacklistController.checkForPhishing('zero.metamask.io') + assert.equal(result, false) + }) + it('should not flag the mascara-faucet domain', function () { + const result = blacklistController.checkForPhishing('zero-faucet.metamask.io') + assert.equal(result, false) + }) + }) +}) \ No newline at end of file diff --git a/test/unit/network-contoller-test.js b/test/unit/network-contoller-test.js index 0c7ee9d70..87c2ee7a3 100644 --- a/test/unit/network-contoller-test.js +++ b/test/unit/network-contoller-test.js @@ -3,6 +3,9 @@ const NetworkController = require('../../app/scripts/controllers/network') describe('# Network Controller', function () { let networkController + const networkControllerProviderInit = { + getAccounts: () => {}, + } beforeEach(function () { networkController = new NetworkController({ @@ -10,26 +13,13 @@ describe('# Network Controller', function () { type: 'rinkeby', }, }) - // stub out provider - networkController._provider = new Proxy({}, { - get: (obj, name) => { - return () => {} - }, - }) - networkController.providerInit = { - getAccounts: () => {}, - } - networkController.ethQuery = new Proxy({}, { - get: (obj, name) => { - return () => {} - }, - }) + networkController.initializeProvider(networkControllerProviderInit, dummyProviderConstructor) }) describe('network', function () { describe('#provider', function () { it('provider should be updatable without reassignment', function () { - networkController.initializeProvider(networkController.providerInit) + networkController.initializeProvider(networkControllerProviderInit, dummyProviderConstructor) const provider = networkController.provider networkController._provider = {test: true} assert.ok(provider.test) @@ -75,3 +65,19 @@ describe('# Network Controller', function () { }) }) }) + +function dummyProviderConstructor() { + return { + // provider + sendAsync: noop, + // block tracker + start: noop, + stop: noop, + on: noop, + addListener: noop, + once: noop, + removeAllListeners: noop, + } +} + +function noop() {} \ No newline at end of file diff --git a/test/unit/nonce-tracker-test.js b/test/unit/nonce-tracker-test.js index 16cd6d008..b0283e159 100644 --- a/test/unit/nonce-tracker-test.js +++ b/test/unit/nonce-tracker-test.js @@ -18,11 +18,13 @@ describe('Nonce Tracker', function () { getPendingTransactions = () => pendingTxs - provider = { sendAsync: (_, cb) => { cb(undefined, {result: '0x0'}) } } - nonceTracker = new NonceTracker({ - blockTracker: { + provider = { + sendAsync: (_, cb) => { cb(undefined, {result: '0x0'}) }, + _blockTracker: { getCurrentBlock: () => '0x11b568', }, + } + nonceTracker = new NonceTracker({ provider, getPendingTransactions, }) diff --git a/test/unit/tx-controller-test.js b/test/unit/tx-controller-test.js index 7b86cfe14..f290088a1 100644 --- a/test/unit/tx-controller-test.js +++ b/test/unit/tx-controller-test.js @@ -1,11 +1,11 @@ const assert = require('assert') const ethUtil = require('ethereumjs-util') const EthTx = require('ethereumjs-tx') -const EthQuery = require('eth-query') const ObservableStore = require('obs-store') const clone = require('clone') const sinon = require('sinon') const TransactionController = require('../../app/scripts/controllers/transactions') +const TxProvideUtils = require('../../app/scripts/lib/tx-utils') const noop = () => true const currentNetworkId = 42 const otherNetworkId = 36 @@ -20,7 +20,6 @@ describe('Transaction Controller', function () { txHistoryLimit: 10, blockTracker: { getCurrentBlock: noop, on: noop, once: noop }, provider: { sendAsync: noop }, - ethQuery: new EthQuery({ sendAsync: noop }), ethStore: { getState: noop }, signTransaction: (ethTx) => new Promise((resolve) => { ethTx.sign(privKey) @@ -28,24 +27,147 @@ describe('Transaction Controller', function () { }), }) txController.nonceTracker.getNonceLock = () => Promise.resolve({ nextNonce: 0, releaseLock: noop }) + txController.query = new Proxy({}, { + get: (queryStubResult, key) => { + if (key === 'stubResult') { + return function (method, ...args) { + queryStubResult[method] = args + } + } else { + const returnValues = queryStubResult[key] + return () => Promise.resolve(...returnValues) + } + }, + }) + txController.txProviderUtils = new TxProvideUtils(txController.query) + }) + + describe('#newUnapprovedTransaction', function () { + let stub, txMeta, txParams + beforeEach(function () { + txParams = { + 'from':'0xc684832530fcbddae4b4230a47e991ddcec2831d', + 'to':'0xc684832530fcbddae4b4230a47e991ddcec2831d', + }, + txMeta = { + status: 'unapproved', + id: 1, + metamaskNetworkId: currentNetworkId, + txParams, + } + txController._saveTxList([txMeta]) + stub = sinon.stub(txController, 'addUnapprovedTransaction').returns(Promise.resolve(txMeta)) + }) + + afterEach(function () { + stub.restore() + }) + + it('should emit newUnaprovedTx event and pass txMeta as the first argument', function (done) { + txController.once('newUnaprovedTx', (txMetaFromEmit) => { + assert(txMetaFromEmit, 'txMeta is falsey') + assert.equal(txMetaFromEmit.id, 1, 'the right txMeta was passed') + done() + }) + txController.newUnapprovedTransaction(txParams) + .catch(done) + }) + + it('should resolve when finished and status is submitted and resolve with the hash', function (done) { + txController.once('newUnaprovedTx', (txMetaFromEmit) => { + setTimeout(() => { + console.log('HELLLO') + txController.setTxHash(txMetaFromEmit.id, '0x0') + txController.setTxStatusSubmitted(txMetaFromEmit.id) + }, 10) + }) + + txController.newUnapprovedTransaction(txParams) + .then((hash) => { + assert(hash, 'newUnapprovedTransaction needs to return the hash') + done() + }) + .catch(done) + }) + + it('should reject when finished and status is rejected', function (done) { + txController.once('newUnaprovedTx', (txMetaFromEmit) => { + setTimeout(() => { + console.log('HELLLO') + txController.setTxStatusRejected(txMetaFromEmit.id) + }, 10) + }) + + txController.newUnapprovedTransaction(txParams) + .catch((err) => { + if (err.message === 'MetaMask Tx Signature: User denied transaction signature.') done() + else done(err) + }) + }) + }) + + describe('#addUnapprovedTransaction', function () { + it('should add an unapproved transaction and return a valid txMeta', function (done) { + const addTxDefaultsStub = sinon.stub(txController, 'addTxDefaults').callsFake(() => Promise.resolve()) + txController.addUnapprovedTransaction({}) + .then((txMeta) => { + assert(('id' in txMeta), 'should have a id') + assert(('time' in txMeta), 'should have a time stamp') + assert(('metamaskNetworkId' in txMeta), 'should have a metamaskNetworkId') + assert(('txParams' in txMeta), 'should have a txParams') + assert(('history' in txMeta), 'should have a history') + + const memTxMeta = txController.getTx(txMeta.id) + assert.deepEqual(txMeta, memTxMeta, `txMeta should be stored in txController after adding it\n expected: ${txMeta} \n got: ${memTxMeta}`) + addTxDefaultsStub.restore() + done() + }).catch(done) + }) + }) + + describe('#addTxDefaults', function () { + it('should add the tx defaults if their are none', function (done) { + let txMeta = { + 'txParams': { + 'from':'0xc684832530fcbddae4b4230a47e991ddcec2831d', + 'to':'0xc684832530fcbddae4b4230a47e991ddcec2831d', + }, + } + + txController.query.stubResult('gasPrice', '0x4a817c800') + txController.query.stubResult('getBlockByNumber', { gasLimit: '0x47b784' }) + txController.query.stubResult('estimateGas', '0x5209') + + txController.addTxDefaults(txMeta) + .then((txMetaWithDefaults) => { + assert(txMetaWithDefaults.txParams.value, '0x0','should have added 0x0 as the value') + assert(txMetaWithDefaults.txParams.gasPrice, 'should have added the gas price') + assert(txMetaWithDefaults.txParams.gas, 'should have added the gas field') + done() + }) + .catch(done) + }) }) describe('#validateTxParams', function () { - it('returns null for positive values', function () { + it('does not throw for positive values', function (done) { var sample = { value: '0x01', } - txController.txProviderUtils.validateTxParams(sample, (err) => { - assert.equal(err, null, 'no error') - }) + txController.txProviderUtils.validateTxParams(sample).then(() => { + done() + }).catch(done) }) - it('returns error for negative values', function () { + it('returns error for negative values', function (done) { var sample = { value: '-0x01', } - txController.txProviderUtils.validateTxParams(sample, (err) => { + txController.txProviderUtils.validateTxParams(sample) + .then(() => done('expected to thrown on negativity values but didn\'t')) + .catch((err) => { assert.ok(err, 'error') + done() }) }) }) @@ -56,9 +178,6 @@ describe('Transaction Controller', function () { assert.ok(Array.isArray(result)) assert.equal(result.length, 0) }) - it('should also return transactions from local storage if any', function () { - - }) }) describe('#addTx', function () { @@ -276,16 +395,15 @@ describe('Transaction Controller', function () { txController.addTx(txMeta) - const estimateStub = sinon.stub(txController.txProviderUtils.query, 'estimateGas') - .callsArgWithAsync(1, null, wrongValue) + txController.query.stubResult('estimateGas', wrongValue) + txController.query.stubResult('gasPrice', wrongValue) - const priceStub = sinon.stub(txController.txProviderUtils.query, 'gasPrice') - .callsArgWithAsync(0, null, wrongValue) + const signStub = sinon.stub(txController, 'signTransaction').callsFake(() => Promise.resolve()) - - const signStub = sinon.stub(txController, 'signTransaction', () => Promise.resolve()) - - const pubStub = sinon.stub(txController.txProviderUtils, 'publishTransaction', () => Promise.resolve(originalValue)) + const pubStub = sinon.stub(txController, 'publishTransaction').callsFake(() => { + txController.setTxHash('1', originalValue) + txController.setTxStatusSubmitted('1') + }) txController.approveTransaction(txMeta.id).then(() => { const result = txController.getTx(txMeta.id) @@ -294,9 +412,6 @@ describe('Transaction Controller', function () { assert.equal(params.gas, originalValue, 'gas unmodified') assert.equal(params.gasPrice, originalValue, 'gas price unmodified') assert.equal(result.hash, originalValue, `hash was set \n got: ${result.hash} \n expected: ${originalValue}`) - - estimateStub.restore() - priceStub.restore() signStub.restore() pubStub.restore() done() @@ -343,14 +458,17 @@ describe('Transaction Controller', function () { // Adding the fake tx: txController.addTx(clone(txMeta)) - txController._resubmitTx(txMeta, function (err) { - assert.ifError(err, 'should not throw an error') + txController._resubmitTx(txMeta) + .then(() => { const updatedMeta = txController.getTx(txMeta.id) assert.notEqual(updatedMeta.status, txMeta.status, 'status changed.') assert.equal(updatedMeta.status, 'failed', 'tx set to failed.') done() }) + .catch((err) => { + assert.ifError(err, 'should not throw an error') + done(err) + }) }) }) -}) - +}) \ No newline at end of file diff --git a/ui/app/components/pending-tx.js b/ui/app/components/pending-tx.js index d7d602f31..5324ccd64 100644 --- a/ui/app/components/pending-tx.js +++ b/ui/app/components/pending-tx.js @@ -15,7 +15,7 @@ const addressSummary = util.addressSummary const nameForAddress = require('../../lib/contract-namer') const BNInput = require('./bn-as-decimal-input') -const MIN_GAS_PRICE_GWEI_BN = new BN(2) +const MIN_GAS_PRICE_GWEI_BN = new BN(1) const GWEI_FACTOR = new BN(1e9) const MIN_GAS_PRICE_BN = MIN_GAS_PRICE_GWEI_BN.mul(GWEI_FACTOR) const MIN_GAS_LIMIT_BN = new BN(21000) diff --git a/ui/app/reducers.js b/ui/app/reducers.js index 11efca529..36045772f 100644 --- a/ui/app/reducers.js +++ b/ui/app/reducers.js @@ -43,7 +43,6 @@ function rootReducer (state, action) { window.logState = function () { var stateString = JSON.stringify(window.METAMASK_CACHED_LOG_STATE, removeSeedWords, 2) - console.log(stateString) return stateString } From e223b0c7898ad49bd3df044ff72386869b0702be Mon Sep 17 00:00:00 2001 From: kumavis Date: Thu, 3 Aug 2017 16:34:38 -0700 Subject: [PATCH 06/38] test - dont mix callback and promise --- test/unit/nonce-tracker-test.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/unit/nonce-tracker-test.js b/test/unit/nonce-tracker-test.js index b0283e159..36025a360 100644 --- a/test/unit/nonce-tracker-test.js +++ b/test/unit/nonce-tracker-test.js @@ -31,12 +31,11 @@ describe('Nonce Tracker', function () { }) describe('#getNonceLock', function () { - it('should work', async function (done) { + it('should work', async function () { this.timeout(15000) const nonceLock = await nonceTracker.getNonceLock('0x7d3517b0d011698406d6e0aed8453f0be2697926') assert.equal(nonceLock.nextNonce, '1', 'nonce should be 1') await nonceLock.releaseLock() - done() }) }) }) From b106fa814bfdd5576199969afa6302cda1274c49 Mon Sep 17 00:00:00 2001 From: kumavis Date: Thu, 3 Aug 2017 16:35:54 -0700 Subject: [PATCH 07/38] README - fix bad merge --- README.md | 4 ---- 1 file changed, 4 deletions(-) diff --git a/README.md b/README.md index ca6437a0e..ddada7ec9 100644 --- a/README.md +++ b/README.md @@ -4,10 +4,6 @@ ## Support If you're a user seeking support, [here is our support site](http://metamask.consensyssupport.happyfox.com). -<<<<<<< HEAD - -======= ->>>>>>> b1311b48c96d873f571e26cad8e4f5d2f61dec25 ## Developing Compatible Dapps From 55f0187df7c1dcde9a63d11cbfc152e936c2a9fa Mon Sep 17 00:00:00 2001 From: kumavis Date: Thu, 3 Aug 2017 16:38:44 -0700 Subject: [PATCH 08/38] deps - update --- package.json | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/package.json b/package.json index cbfcd3a76..2743c4e52 100644 --- a/package.json +++ b/package.json @@ -57,7 +57,7 @@ "browserify-derequire": "^0.9.4", "client-sw-ready-event": "^3.3.0", "clone": "^2.1.1", - "copy-to-clipboard": "^3.0.6", + "copy-to-clipboard": "^3.0.8", "debounce": "^1.0.0", "deep-extend": "^0.5.0", "detect-node": "^2.0.3", @@ -74,7 +74,7 @@ "eth-simple-keyring": "^1.1.1", "eth-token-tracker": "^1.1.2", "ethereumjs-tx": "^1.3.0", - "ethereumjs-util": "ethereumjs/ethereumjs-util#ac5d0908536b447083ea422b435da27f26615de9", + "ethereumjs-util": "github:ethereumjs/ethereumjs-util#ac5d0908536b447083ea422b435da27f26615de9", "ethereumjs-wallet": "^0.6.0", "ethjs-ens": "^2.0.0", "ethjs-query": "^0.2.6", @@ -82,6 +82,7 @@ "extension-link-enabler": "^1.0.0", "extensionizer": "^1.0.0", "fast-levenshtein": "^2.0.6", + "gulp": "github:gulpjs/gulp#4.0", "gulp-eslint": "^4.0.0", "hat": "0.0.3", "idb-global": "^2.1.0", @@ -126,10 +127,10 @@ "semaphore": "^1.0.5", "sw-stream": "^2.0.0", "textarea-caret": "^3.0.1", + "through2": "^2.0.3", "valid-url": "^1.0.9", "vreme": "^3.0.2", - "web3": "0.20.1", - "through2": "^2.0.3", + "web3": "^0.20.1", "web3-provider-engine": "^13.2.9", "web3-stream-provider": "^3.0.1", "xtend": "^4.0.1" From bbb4c36234e6f392b855b776ca5ca1f1628bcdf5 Mon Sep 17 00:00:00 2001 From: kumavis Date: Thu, 3 Aug 2017 16:40:56 -0700 Subject: [PATCH 09/38] readme - fix greenkeeper badge position --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index ddada7ec9..e39058790 100644 --- a/README.md +++ b/README.md @@ -1,5 +1,5 @@ -# MetaMask Plugin [![Build Status](https://circleci.com/gh/MetaMask/metamask-extension.svg?style=shield&circle-token=a1ddcf3cd38e29267f254c9c59d556d513e3a1fd)](https://circleci.com/gh/MetaMask/metamask-extension) [![Coverage Status](https://coveralls.io/repos/github/MetaMask/metamask-extension/badge.svg?branch=master)](https://coveralls.io/github/MetaMask/metamask-extension?branch=master) -[![Greenkeeper badge](https://badges.greenkeeper.io/MetaMask/metamask-extension.svg)](https://greenkeeper.io/) +# MetaMask Plugin [![Build Status](https://circleci.com/gh/MetaMask/metamask-extension.svg?style=shield&circle-token=a1ddcf3cd38e29267f254c9c59d556d513e3a1fd)](https://circleci.com/gh/MetaMask/metamask-extension) [![Coverage Status](https://coveralls.io/repos/github/MetaMask/metamask-extension/badge.svg?branch=master)](https://coveralls.io/github/MetaMask/metamask-extension?branch=master) [![Greenkeeper badge](https://badges.greenkeeper.io/MetaMask/metamask-extension.svg)](https://greenkeeper.io/) + ## Support From 3dcc199845f4026cc4d02f9f760332ac9752ff69 Mon Sep 17 00:00:00 2001 From: frankiebee Date: Fri, 4 Aug 2017 13:27:08 -0400 Subject: [PATCH 10/38] rename the test description Transaction Manger -> Message Manger --- test/unit/message-manager-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/message-manager-test.js b/test/unit/message-manager-test.js index 30cb4f067..9b76241ed 100644 --- a/test/unit/message-manager-test.js +++ b/test/unit/message-manager-test.js @@ -1,7 +1,7 @@ const assert = require('assert') const MessageManger = require('../../app/scripts/lib/message-manager') -describe('Transaction Manager', function () { +describe('Message Manager', function () { let messageManager beforeEach(function () { From caee2a9e35c0e80efed9da0798cb75044db6c920 Mon Sep 17 00:00:00 2001 From: frankiebee Date: Fri, 4 Aug 2017 13:55:00 -0400 Subject: [PATCH 11/38] move util functions to util.js --- app/scripts/lib/tx-utils.js | 41 ++++++++----------------------------- app/scripts/lib/util.js | 31 ++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 32 deletions(-) diff --git a/app/scripts/lib/tx-utils.js b/app/scripts/lib/tx-utils.js index 3687a9652..a2db4abd8 100644 --- a/app/scripts/lib/tx-utils.js +++ b/app/scripts/lib/tx-utils.js @@ -1,7 +1,11 @@ -const ethUtil = require('ethereumjs-util') +const EthQuery = require('ethjs-query') const Transaction = require('ethereumjs-tx') const normalize = require('eth-sig-util').normalize -const BN = ethUtil.BN +const { + hexToBn, + BnMultiplyByFraction, + bnToHex, +} = require('./util') /* tx-utils are utility methods for Transaction manager @@ -10,8 +14,8 @@ and used to do things like calculate gas of a tx. */ module.exports = class txProvideUtils { - constructor (ethQuery) { - this.query = ethQuery + constructor (provider) { + this.query = new EthQuery(provider) } async analyzeGasUsage (txMeta) { @@ -91,31 +95,4 @@ module.exports = class txProvideUtils { throw new Error(`Invalid transaction value of ${txParams.value} not a positive number.`) } } - - sufficientBalance (txParams, hexBalance) { - const balance = hexToBn(hexBalance) - const value = hexToBn(txParams.value) - const gasLimit = hexToBn(txParams.gas) - const gasPrice = hexToBn(txParams.gasPrice) - - const maxCost = value.add(gasLimit.mul(gasPrice)) - return balance.gte(maxCost) - } - -} - -// util - -function bnToHex (inputBn) { - return ethUtil.addHexPrefix(inputBn.toString(16)) -} - -function hexToBn (inputHex) { - return new BN(ethUtil.stripHexPrefix(inputHex), 16) -} - -function BnMultiplyByFraction (targetBN, numerator, denominator) { - const numBN = new BN(numerator) - const denomBN = new BN(denominator) - return targetBN.mul(numBN).div(denomBN) -} +} \ No newline at end of file diff --git a/app/scripts/lib/util.js b/app/scripts/lib/util.js index bddd60ee8..70390e95c 100644 --- a/app/scripts/lib/util.js +++ b/app/scripts/lib/util.js @@ -1,8 +1,39 @@ +const ethUtil = require('ethereumjs-util') +const BN = ethUtil.BN + module.exports = { getStack, + sufficientBalance, + hexToBn, + bnToHex, + BnMultiplyByFraction, } function getStack () { const stack = new Error('Stack trace generator - not an error').stack return stack } + +function sufficientBalance (txParams, hexBalance) { + const balance = hexToBn(hexBalance) + const value = hexToBn(txParams.value) + const gasLimit = hexToBn(txParams.gas) + const gasPrice = hexToBn(txParams.gasPrice) + + const maxCost = value.add(gasLimit.mul(gasPrice)) + return balance.gte(maxCost) +} + +function bnToHex (inputBn) { + return ethUtil.addHexPrefix(inputBn.toString(16)) +} + +function hexToBn (inputHex) { + return new BN(ethUtil.stripHexPrefix(inputHex), 16) +} + +function BnMultiplyByFraction (targetBN, numerator, denominator) { + const numBN = new BN(numerator) + const denomBN = new BN(denominator) + return targetBN.mul(numBN).div(denomBN) +} From 4ac0b972025ddd55a9a42b4a15db0a5f207f4078 Mon Sep 17 00:00:00 2001 From: frankiebee Date: Fri, 4 Aug 2017 14:36:27 -0400 Subject: [PATCH 12/38] test for SufficientBalance --- test/unit/util-test.js | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 test/unit/util-test.js diff --git a/test/unit/util-test.js b/test/unit/util-test.js new file mode 100644 index 000000000..7716e4872 --- /dev/null +++ b/test/unit/util-test.js @@ -0,0 +1,41 @@ +const assert = require('assert') +const { sufficientBalance } = require('../../app/scripts/lib/util') + + +describe.only('SufficientBalance', function () { + it('returns true if max tx cost is equal to balance.', function () { + const tx = { + 'value': '0x1', + 'gas': '0x2', + 'gasPrice': '0x3', + } + const balance = '0x8' + + const result = sufficientBalance(tx, balance) + assert.ok(result, 'sufficient balance found.') + }) + + it('returns true if max tx cost is less than balance.', function () { + const tx = { + 'value': '0x1', + 'gas': '0x2', + 'gasPrice': '0x3', + } + const balance = '0x9' + + const result = sufficientBalance(tx, balance) + assert.ok(result, 'sufficient balance found.') + }) + + it('returns false if max tx cost is more than balance.', function () { + const tx = { + 'value': '0x1', + 'gas': '0x2', + 'gasPrice': '0x3', + } + const balance = '0x6' + + const result = sufficientBalance(tx, balance) + assert.ok(!result, 'insufficient balance found.') + }) +}) \ No newline at end of file From 54739cb798b70e14d7774d9e92745188afca5461 Mon Sep 17 00:00:00 2001 From: frankiebee Date: Fri, 4 Aug 2017 14:40:22 -0400 Subject: [PATCH 13/38] test for pending tx watcher --- test/unit/pending-tx-test.js | 271 ++++++++++++++++++++++++++++++++ test/unit/tx-controller-test.js | 49 +----- test/unit/tx-utils-test.js | 58 +------ test/unit/util-test.js | 2 +- 4 files changed, 280 insertions(+), 100 deletions(-) create mode 100644 test/unit/pending-tx-test.js diff --git a/test/unit/pending-tx-test.js b/test/unit/pending-tx-test.js new file mode 100644 index 000000000..5103a9b62 --- /dev/null +++ b/test/unit/pending-tx-test.js @@ -0,0 +1,271 @@ +const assert = require('assert') +const ethUtil = require('ethereumjs-util') +const EthTx = require('ethereumjs-tx') +const ObservableStore = require('obs-store') +const clone = require('clone') +const PendingTransactionWatcher = require('../../app/scripts/lib/pending-tx-watchers') +const noop = () => true +const currentNetworkId = 42 +const otherNetworkId = 36 +const privKey = new Buffer('8718b9618a37d1fc78c436511fc6df3c8258d3250635bba617f33003270ec03e', 'hex') + +describe('PendingTransactionWatcher', function () { + let pendingTxWatcher, txMeta, txMetaNoHash, txMetaNoRawTx + this.timeout(10000) + beforeEach(function () { + txMeta = { + id: 1, + hash: '0x0593ee121b92e10d63150ad08b4b8f9c7857d1bd160195ee648fb9a0f8d00eebBlock', + status: 'signed', + txParams: { + from: '0x1678a085c290ebd122dc42cba69373b5953b831d', + nonce: '0x1', + value: '0xfffff', + }, + rawTx: '0xf86c808504a817c800827b0d940c62bb85faa3311a998d3aba8098c1235c564966880de0b6b3a7640000802aa08ff665feb887a25d4099e40e11f0fef93ee9608f404bd3f853dd9e84ed3317a6a02ec9d3d1d6e176d4d2593dd760e74ccac753e6a0ea0d00cc9789d0d7ff1f471d', + } + txMetaNoHash = { + id: 2, + status: 'signed', + txParams: { from: '0x1678a085c290ebd122dc42cba69373b5953b831d'}, + } + txMetaNoRawTx = { + hash: '0x0593ee121b92e10d63150ad08b4b8f9c7857d1bd160195ee648fb9a0f8d00eebBlock', + status: 'signed', + txParams: { from: '0x1678a085c290ebd122dc42cba69373b5953b831d'}, + } + + pendingTxWatcher = new PendingTransactionWatcher({ + provider: { sendAsync: noop }, + getBalance: () => {}, + nonceTracker: { + getGlobalLock: async () => { + return { releaseLock: () => {} } + } + }, + getPendingTransactions: () => {return []}, + sufficientBalance: () => {}, + publishTransaction: () => {}, + }) + + pendingTxWatcher.query = new Proxy({}, { + get: (queryStubResult, key) => { + if (key === 'stubResult') { + return function (method, ...args) { + queryStubResult[method] = args + } + } else { + const returnValues = queryStubResult[key] + return () => Promise.resolve(...returnValues) + } + }, + }) + + }) + + describe('#checkForTxInBlock', function () { + it('should return if no pending transactions', function () { + // throw a type error if it trys to do anything on the block + // thus failing the test + const block = Proxy.revocable({}, {}).revoke() + pendingTxWatcher.checkForTxInBlock(block) + }) + it('should emit \'txFailed\' if the txMeta does not have a hash', function (done) { + const block = Proxy.revocable({}, {}).revoke() + pendingTxWatcher.getPendingTransactions = () => [txMetaNoHash] + pendingTxWatcher.once('txFailed', (txId, err) => { + assert(txId, txMetaNoHash.id, 'should pass txId') + done() + }) + pendingTxWatcher.checkForTxInBlock(block) + }) + it('should emit \'txConfirmed\' if the tx is in the block', function (done) { + const block = { transactions: [txMeta]} + pendingTxWatcher.getPendingTransactions = () => [txMeta] + pendingTxWatcher.once('txConfirmed', (txId) => { + assert(txId, txMeta.id, 'should pass txId') + done() + }) + pendingTxWatcher.once('txFailed', (_, err) => { done(err) }) + pendingTxWatcher.checkForTxInBlock(block) + }) + }) + describe('#queryPendingTxs', function () { + it('should call #_checkPendingTxs if their is no oldBlock', function (done) { + let newBlock, oldBlock + newBlock = { number: '0x01' } + pendingTxWatcher._checkPendingTxs = done + pendingTxWatcher.queryPendingTxs({oldBlock, newBlock}) + }) + it('should call #_checkPendingTxs if oldBlock and the newBlock have a diff of greater then 1', function (done) { + let newBlock, oldBlock + oldBlock = { number: '0x01' } + newBlock = { number: '0x03' } + pendingTxWatcher._checkPendingTxs = done + pendingTxWatcher.queryPendingTxs({oldBlock, newBlock}) + }) + it('should not call #_checkPendingTxs if oldBlock and the newBlock have a diff of 1 or less', function (done) { + let newBlock, oldBlock + oldBlock = { number: '0x1' } + newBlock = { number: '0x2' } + pendingTxWatcher._checkPendingTxs = () => { + const err = new Error('should not call #_checkPendingTxs if oldBlock and the newBlock have a diff of 1 or less') + done(err) + } + pendingTxWatcher.queryPendingTxs({oldBlock, newBlock}) + done() + }) + }) + + describe('#_checkPendingTx', function () { + it('should emit \'txFailed\' if the txMeta does not have a hash', function (done) { + pendingTxWatcher.once('txFailed', (txId, err) => { + assert(txId, txMetaNoHash.id, 'should pass txId') + done() + }) + pendingTxWatcher._checkPendingTx(txMetaNoHash) + }) + + it('should should return if query does not return txParams', function () { + pendingTxWatcher.query.stubResult('getTransactionByHash', null) + pendingTxWatcher._checkPendingTx(txMeta) + }) + + it('should emit \'txConfirmed\'', function (done) { + pendingTxWatcher.query.stubResult('getTransactionByHash', {blockNumber: '0x01'}) + pendingTxWatcher.once('txConfirmed', (txId) => { + assert(txId, txMeta.id, 'should pass txId') + done() + }) + pendingTxWatcher.once('txFailed', (_, err) => { done(err) }) + pendingTxWatcher._checkPendingTx(txMeta) + }) + }) + + describe('#_checkPendingTxs', function () { + beforeEach(function () { + const txMeta2 = txMeta3 = txMeta + txMeta2.id = 2 + txMeta3.id = 3 + txList = [txMeta, txMeta2, txMeta3].map((tx) => { + tx.processed = new Promise ((resolve) => { tx.resolve = resolve }) + return tx + }) + }) + + it('should warp all txMeta\'s in #_checkPendingTx', function (done) { + pendingTxWatcher.getPendingTransactions = () => txList + pendingTxWatcher._checkPendingTx = (tx) => { tx.resolve(tx) } + const list = txList.map + Promise.all(txList.map((tx) => tx.processed)) + .then((txCompletedList) => done()) + .catch(done) + + pendingTxWatcher._checkPendingTxs() + }) + }) + + describe('#resubmitPendingTxs', function () { + beforeEach(function () { + const txMeta2 = txMeta3 = txMeta + txList = [txMeta, txMeta2, txMeta3].map((tx) => { + tx.processed = new Promise ((resolve) => { tx.resolve = resolve }) + return tx + }) + }) + + it('should return if no pending transactions', function () { + pendingTxWatcher.resubmitPendingTxs() + }) + it('should call #_resubmitTx for all pending tx\'s', function (done) { + pendingTxWatcher.getPendingTransactions = () => txList + pendingTxWatcher._resubmitTx = async (tx) => { tx.resolve(tx) } + Promise.all(txList.map((tx) => tx.processed)) + .then((txCompletedList) => done()) + .catch(done) + pendingTxWatcher.resubmitPendingTxs() + }) + it('should not emit \'txFailed\' if the txMeta throws a known txError', function (done) { + knownErrors =[ + // geth + ' Replacement transaction Underpriced ', + ' known transaction', + // parity + 'Gas price too low to replace ', + ' transaction with the sAme hash was already imported', + // other + ' gateway timeout', + ' noncE too low ', + ] + const enoughForAllErrors = txList.concat(txList) + + pendingTxWatcher.on('txFailed', (_, err) => done(err)) + + pendingTxWatcher.getPendingTransactions = () => enoughForAllErrors + pendingTxWatcher._resubmitTx = async (tx) => { + tx.resolve() + throw new Error(knownErrors.pop()) + } + Promise.all(txList.map((tx) => tx.processed)) + .then((txCompletedList) => done()) + .catch(done) + + pendingTxWatcher.resubmitPendingTxs() + }) + it('should emit \'txFailed\' if it encountered a real error', function (done) { + pendingTxWatcher.once('txFailed', (id, err) => err.message === 'im some real error' ? txList[id - 1].resolve() : done(err)) + + pendingTxWatcher.getPendingTransactions = () => txList + pendingTxWatcher._resubmitTx = async (tx) => { throw new TypeError('im some real error') } + Promise.all(txList.map((tx) => tx.processed)) + .then((txCompletedList) => done()) + .catch(done) + + pendingTxWatcher.resubmitPendingTxs() + }) + }) + describe('#_resubmitTx with a too-low balance', function () { + it('should return before publishing the transaction because to low of balance', function (done) { + const lowBalance = '0x0' + pendingTxWatcher.getBalance = (address) => { + assert.equal(address, txMeta.txParams.from, 'Should pass the address') + return lowBalance + } + pendingTxWatcher.publishTransaction = async (rawTx) => { + done(new Error('tried to publish transaction')) + } + + // Stubbing out current account state: + // Adding the fake tx: + pendingTxWatcher.once('txWarning', (txMeta) => { + assert(txMeta.warning.message, 'Should have a warning message') + done() + }) + pendingTxWatcher._resubmitTx(txMeta) + .catch((err) => { + assert.ifError(err, 'should not throw an error') + done(err) + }) + }) + + it('should publishing the transaction', function (done) { + const enoughBalance = '0x100000' + pendingTxWatcher.getBalance = (address) => { + assert.equal(address, txMeta.txParams.from, 'Should pass the address') + return enoughBalance + } + pendingTxWatcher.publishTransaction = async (rawTx) => { + assert.equal(rawTx, txMeta.rawTx, 'Should pass the rawTx') + } + + // Stubbing out current account state: + // Adding the fake tx: + pendingTxWatcher._resubmitTx(txMeta) + .then(() => done()) + .catch((err) => { + assert.ifError(err, 'should not throw an error') + done(err) + }) + }) + }) +}) \ No newline at end of file diff --git a/test/unit/tx-controller-test.js b/test/unit/tx-controller-test.js index f290088a1..e54dc9719 100644 --- a/test/unit/tx-controller-test.js +++ b/test/unit/tx-controller-test.js @@ -27,7 +27,8 @@ describe('Transaction Controller', function () { }), }) txController.nonceTracker.getNonceLock = () => Promise.resolve({ nextNonce: 0, releaseLock: noop }) - txController.query = new Proxy({}, { + txController.txProviderUtils = new TxProvideUtils(txController.provider) + txController.query = txController.txProviderUtils.query = new Proxy({}, { get: (queryStubResult, key) => { if (key === 'stubResult') { return function (method, ...args) { @@ -39,7 +40,7 @@ describe('Transaction Controller', function () { } }, }) - txController.txProviderUtils = new TxProvideUtils(txController.query) + }) describe('#newUnapprovedTransaction', function () { @@ -76,7 +77,6 @@ describe('Transaction Controller', function () { it('should resolve when finished and status is submitted and resolve with the hash', function (done) { txController.once('newUnaprovedTx', (txMetaFromEmit) => { setTimeout(() => { - console.log('HELLLO') txController.setTxHash(txMetaFromEmit.id, '0x0') txController.setTxStatusSubmitted(txMetaFromEmit.id) }, 10) @@ -93,7 +93,6 @@ describe('Transaction Controller', function () { it('should reject when finished and status is rejected', function (done) { txController.once('newUnaprovedTx', (txMetaFromEmit) => { setTimeout(() => { - console.log('HELLLO') txController.setTxStatusRejected(txMetaFromEmit.id) }, 10) }) @@ -429,46 +428,4 @@ describe('Transaction Controller', function () { }).catch(done) }) }) - - describe('#_resubmitTx with a too-low balance', function () { - it('should fail the transaction', function (done) { - const from = '0xda0da0' - const txMeta = { - id: 1, - status: 'submitted', - metamaskNetworkId: currentNetworkId, - txParams: { - from, - nonce: '0x1', - value: '0xfffff', - }, - } - - const lowBalance = '0x0' - const fakeStoreState = { accounts: {} } - fakeStoreState.accounts[from] = { - balance: lowBalance, - nonce: '0x0', - } - - // Stubbing out current account state: - const getStateStub = sinon.stub(txController.ethStore, 'getState') - .returns(fakeStoreState) - - // Adding the fake tx: - txController.addTx(clone(txMeta)) - - txController._resubmitTx(txMeta) - .then(() => { - const updatedMeta = txController.getTx(txMeta.id) - assert.notEqual(updatedMeta.status, txMeta.status, 'status changed.') - assert.equal(updatedMeta.status, 'failed', 'tx set to failed.') - done() - }) - .catch((err) => { - assert.ifError(err, 'should not throw an error') - done(err) - }) - }) - }) }) \ No newline at end of file diff --git a/test/unit/tx-utils-test.js b/test/unit/tx-utils-test.js index a43bcfb35..43128b977 100644 --- a/test/unit/tx-utils-test.js +++ b/test/unit/tx-utils-test.js @@ -1,7 +1,7 @@ const assert = require('assert') -const ethUtil = require('ethereumjs-util') -const BN = ethUtil.BN +const BN = require('bn.js') +const { hexToBn, bnToHex } = require('../../app/scripts/lib/util') const TxUtils = require('../../app/scripts/lib/tx-utils') @@ -16,44 +16,6 @@ describe('txUtils', function () { })) }) - describe('#sufficientBalance', function () { - it('returns true if max tx cost is equal to balance.', function () { - const tx = { - 'value': '0x1', - 'gas': '0x2', - 'gasPrice': '0x3', - } - const balance = '0x8' - - const result = txUtils.sufficientBalance(tx, balance) - assert.ok(result, 'sufficient balance found.') - }) - - it('returns true if max tx cost is less than balance.', function () { - const tx = { - 'value': '0x1', - 'gas': '0x2', - 'gasPrice': '0x3', - } - const balance = '0x9' - - const result = txUtils.sufficientBalance(tx, balance) - assert.ok(result, 'sufficient balance found.') - }) - - it('returns false if max tx cost is more than balance.', function () { - const tx = { - 'value': '0x1', - 'gas': '0x2', - 'gasPrice': '0x3', - } - const balance = '0x6' - - const result = txUtils.sufficientBalance(tx, balance) - assert.ok(!result, 'insufficient balance found.') - }) - }) - describe('chain Id', function () { it('prepares a transaction with the provided chainId', function () { const txParams = { @@ -96,7 +58,7 @@ describe('txUtils', function () { assert(outputBn.eq(expectedBn), 'returns the original estimatedGas value') }) - it('buffers up to reccomend gas limit reccomended ceiling', function () { + it('buffers up to recommend gas limit recommended ceiling', function () { // naive estimatedGas: 0x16e360 (1.5 mil) const inputHex = '0x16e360' // dummy gas limit: 0x1e8480 (2 mil) @@ -107,17 +69,7 @@ describe('txUtils', function () { // const inputBn = hexToBn(inputHex) // const outputBn = hexToBn(output) const expectedHex = bnToHex(ceilGasLimitBn) - assert.equal(output, expectedHex, 'returns the gas limit reccomended ceiling value') + assert.equal(output, expectedHex, 'returns the gas limit recommended ceiling value') }) }) -}) - -// util - -function hexToBn (inputHex) { - return new BN(ethUtil.stripHexPrefix(inputHex), 16) -} - -function bnToHex (inputBn) { - return ethUtil.addHexPrefix(inputBn.toString(16)) -} +}) \ No newline at end of file diff --git a/test/unit/util-test.js b/test/unit/util-test.js index 7716e4872..6da185b2c 100644 --- a/test/unit/util-test.js +++ b/test/unit/util-test.js @@ -2,7 +2,7 @@ const assert = require('assert') const { sufficientBalance } = require('../../app/scripts/lib/util') -describe.only('SufficientBalance', function () { +describe('SufficientBalance', function () { it('returns true if max tx cost is equal to balance.', function () { const tx = { 'value': '0x1', From 087cd9fb1a42cb59579b8e24804583d6d127e901 Mon Sep 17 00:00:00 2001 From: frankiebee Date: Fri, 4 Aug 2017 14:41:35 -0400 Subject: [PATCH 14/38] break out tx status pendding watchers --- app/scripts/controllers/transactions.js | 172 ++++-------------------- app/scripts/lib/pending-tx-watchers.js | 165 +++++++++++++++++++++++ 2 files changed, 192 insertions(+), 145 deletions(-) create mode 100644 app/scripts/lib/pending-tx-watchers.js diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index d4f32e049..a652c3278 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -5,6 +5,7 @@ const ObservableStore = require('obs-store') const ethUtil = require('ethereumjs-util') const EthQuery = require('ethjs-query') const TxProviderUtil = require('../lib/tx-utils') +const PendingTransactionUtils = require('../lib/pending-tx-watchers') const getStack = require('../lib/util').getStack const createId = require('../lib/random-id') const NonceTracker = require('../lib/nonce-tracker') @@ -21,6 +22,9 @@ module.exports = class TransactionController extends EventEmitter { this.txHistoryLimit = opts.txHistoryLimit this.provider = opts.provider this.blockTracker = opts.blockTracker + this.signEthTx = opts.signTransaction + this.ethStore = opts.ethStore + this.nonceTracker = new NonceTracker({ provider: this.provider, getPendingTransactions: (address) => { @@ -32,15 +36,31 @@ module.exports = class TransactionController extends EventEmitter { }, }) this.query = new EthQuery(this.provider) - this.txProviderUtils = new TxProviderUtil(this.query) - this.blockTracker.on('rawBlock', this.checkForTxInBlock.bind(this)) + this.txProviderUtils = new TxProviderUtil(this.provider) + + this.pendingTxUtils = new PendingTransactionUtils({ + provider: this.provider, + nonceTracker: this.nonceTracker, + getBalance: (address) => this.ethStore.getState().accounts[address].balance, + publishTransaction: this.txProviderUtils.publishTransaction.bind(this.txProviderUtils), + getPendingTransactions: (address) => { + return this.getFilteredTxList({ + from: address, + status: 'submitted', + }) + }, + }) + + this.pendingTxUtils.on('txWarning', this.updateTx.bind(this)) + this.pendingTxUtils.on('txFailed', this.setTxStatusFailed.bind(this)) + this.pendingTxUtils.on('txConfirmed', this.setTxStatusConfirmed.bind(this)) + + this.blockTracker.on('rawBlock', this.pendingTxUtils.checkForTxInBlock.bind(this)) // this is a little messy but until ethstore has been either // removed or redone this is to guard against the race condition // where ethStore hasent been populated by the results yet - this.blockTracker.once('latest', () => this.blockTracker.on('latest', this.resubmitPendingTxs.bind(this))) - this.blockTracker.on('sync', this.queryPendingTxs.bind(this)) - this.signEthTx = opts.signTransaction - this.ethStore = opts.ethStore + this.blockTracker.once('latest', () => this.blockTracker.on('latest', this.pendingTxUtils.resubmitPendingTxs.bind(this))) + this.blockTracker.on('sync', this.pendingTxUtils.queryPendingTxs.bind(this)) // memstore is computed from a few different stores this._updateMemstore() this.store.subscribe(() => this._updateMemstore()) @@ -229,7 +249,7 @@ module.exports = class TransactionController extends EventEmitter { // must set transaction to submitted/failed before releasing lock nonceLock.releaseLock() } catch (err) { - if(!err.message) err.message = 'Transaction failed during approval' + if (!err.message) err.message = 'Transaction failed during approval' this.setTxStatusFailed(txId, err) // must set transaction to submitted/failed before releasing lock if (nonceLock) nonceLock.releaseLock() @@ -374,73 +394,6 @@ module.exports = class TransactionController extends EventEmitter { this.updateTx(txMeta) } - // checks if a signed tx is in a block and - // if included sets the tx status as 'confirmed' - checkForTxInBlock (block) { - const signedTxList = this.getFilteredTxList({status: 'submitted'}) - if (!signedTxList.length) return - signedTxList.forEach((txMeta) => { - const txHash = txMeta.hash - const txId = txMeta.id - - if (!txHash) { - const noTxHashErr = new Error('We had an error while submitting this transaction, please try again.') - noTxHashErr.name = 'NoTxHashError' - this.setTxStatusFailed(txId, noTxHashErr) - } - - - block.transactions.forEach((tx) => { - if (tx.hash === txHash) this.setTxStatusConfirmed(txId) - }) - }) - } - - queryPendingTxs ({oldBlock, newBlock}) { - // check pending transactions on start - if (!oldBlock) { - this._checkPendingTxs() - return - } - // if we synced by more than one block, check for missed pending transactions - const diff = Number.parseInt(newBlock.number) - Number.parseInt(oldBlock.number) - if (diff > 1) this._checkPendingTxs() - } - - resubmitPendingTxs () { - const pending = this.getTxsByMetaData('status', 'submitted') - // only try resubmitting if their are transactions to resubmit - if (!pending.length) return - pending.forEach((txMeta) => this._resubmitTx(txMeta).catch((err) => { - /* - Dont marked as failed if the error is a "known" transaction warning - "there is already a transaction with the same sender-nonce - but higher/same gas price" - */ - const errorMessage = err.message.toLowerCase() - const isKnownTx = ( - // geth - errorMessage.includes('replacement transaction underpriced') - || errorMessage.includes('known transaction') - // parity - || errorMessage.includes('gas price too low to replace') - || errorMessage.includes('transaction with the same hash was already imported') - // other - || errorMessage.includes('gateway timeout') - || errorMessage.includes('nonce too low') - ) - // ignore resubmit warnings, return early - if (isKnownTx) return - // encountered real error - transition to error state - this.setTxStatusFailed(txMeta.id, { - stack: err.stack || err.message, - errCode: err.errCode || err, - message: err.message, - }) - })) - } - - /* _____________________________________ | | | PRIVATE METHODS | @@ -482,75 +435,4 @@ module.exports = class TransactionController extends EventEmitter { }) this.memStore.updateState({ unapprovedTxs, selectedAddressTxList }) } - - async _resubmitTx (txMeta) { - const address = txMeta.txParams.from - const balance = this.ethStore.getState().accounts[address].balance - if (!('retryCount' in txMeta)) txMeta.retryCount = 0 - - // if the value of the transaction is greater then the balance, fail. - if (!this.txProviderUtils.sufficientBalance(txMeta.txParams, balance)) { - const message = 'Insufficient balance.' - this.setTxStatusFailed(txMeta.id, { - stack: '_resubmitTx: custom tx-controller error', - message, - }) - log.error(message) - return - } - - // Only auto-submit already-signed txs: - if (!('rawTx' in txMeta)) return - - // Increment a try counter. - txMeta.retryCount++ - const rawTx = txMeta.rawTx - return await this.txProviderUtils.publishTransaction(rawTx) - } - - // checks the network for signed txs and - // if confirmed sets the tx status as 'confirmed' - async _checkPendingTxs () { - const signedTxList = this.getFilteredTxList({status: 'submitted'}) - // in order to keep the nonceTracker accurate we block it while updating pending transactions - const nonceGlobalLock = await this.nonceTracker.getGlobalLock() - try { - await Promise.all(signedTxList.map((txMeta) => this._checkPendingTx(txMeta))) - } catch (err) { - console.error('TransactionController - Error updating pending transactions') - console.error(err) - } - nonceGlobalLock.releaseLock() - } - - async _checkPendingTx (txMeta) { - const txHash = txMeta.hash - const txId = txMeta.id - // extra check in case there was an uncaught error during the - // signature and submission process - if (!txHash) { - const noTxHashErr = new Error('We had an error while submitting this transaction, please try again.') - noTxHashErr.name = 'NoTxHashError' - this.setTxStatusFailed(txId, noTxHashErr) - } - // get latest transaction status - let txParams - try { - txParams = await this.query.getTransactionByHash(txHash) - if (!txParams) return - if (txParams.blockNumber) { - this.setTxStatusConfirmed(txId) - } - } catch (err) { - if (err || !txParams) { - txMeta.err = { - isWarning: true, - errorCode: err, - message: 'There was a problem loading this transaction.', - } - this.updateTx(txMeta) - throw err - } - } - } } \ No newline at end of file diff --git a/app/scripts/lib/pending-tx-watchers.js b/app/scripts/lib/pending-tx-watchers.js new file mode 100644 index 000000000..4158e8bb5 --- /dev/null +++ b/app/scripts/lib/pending-tx-watchers.js @@ -0,0 +1,165 @@ +const EventEmitter = require('events') +const EthQuery = require('ethjs-query') +const sufficientBalance = require('./util').sufficientBalance +/* + + Utility class for tracking the transactions as they + go from a pending state to a confirmed (mined in a block) state + + As well as continues broadcast while in the pending state + + ~config is not optional~ + requires a: { + provider: //, + nonceTracker: //see nonce tracker, + getBalnce: //(address) a function for getting balances, + getPendingTransactions: //() a function for getting an array of transactions, + publishTransaction: //(rawTx) a async function for publishing raw transactions, + } + +*/ + +module.exports = class PendingTransactionWatcher extends EventEmitter { + constructor (config) { + super() + this.query = new EthQuery(config.provider) + this.nonceTracker = config.nonceTracker + + this.getBalance = config.getBalance + this.getPendingTransactions = config.getPendingTransactions + this.publishTransaction = config.publishTransaction + } + + // checks if a signed tx is in a block and + // if included sets the tx status as 'confirmed' + checkForTxInBlock (block) { + const signedTxList = this.getPendingTransactions() + if (!signedTxList.length) return + signedTxList.forEach((txMeta) => { + const txHash = txMeta.hash + const txId = txMeta.id + + if (!txHash) { + const noTxHashErr = new Error('We had an error while submitting this transaction, please try again.') + noTxHashErr.name = 'NoTxHashError' + this.emit('txFailed', txId, noTxHashErr) + return + } + + + block.transactions.forEach((tx) => { + if (tx.hash === txHash) this.emit('txConfirmed', txId) + }) + }) + } + + queryPendingTxs ({oldBlock, newBlock}) { + // check pending transactions on start + if (!oldBlock) { + this._checkPendingTxs() + return + } + // if we synced by more than one block, check for missed pending transactions + const diff = Number.parseInt(newBlock.number, 16) - Number.parseInt(oldBlock.number, 16) + if (diff > 1) this._checkPendingTxs() + } + + + resubmitPendingTxs () { + const pending = this.getPendingTransactions('status', 'submitted') + // only try resubmitting if their are transactions to resubmit + if (!pending.length) return + pending.forEach((txMeta) => this._resubmitTx(txMeta).catch((err) => { + /* + Dont marked as failed if the error is a "known" transaction warning + "there is already a transaction with the same sender-nonce + but higher/same gas price" + */ + const errorMessage = err.message.toLowerCase() + const isKnownTx = ( + // geth + errorMessage.includes('replacement transaction underpriced') + || errorMessage.includes('known transaction') + // parity + || errorMessage.includes('gas price too low to replace') + || errorMessage.includes('transaction with the same hash was already imported') + // other + || errorMessage.includes('gateway timeout') + || errorMessage.includes('nonce too low') + ) + // ignore resubmit warnings, return early + if (isKnownTx) return + // encountered real error - transition to error state + this.emit('txFailed', txMeta.id, err) + })) + } + + async _resubmitTx (txMeta) { + const address = txMeta.txParams.from + const balance = this.getBalance(address) + if (!('retryCount' in txMeta)) txMeta.retryCount = 0 + + // if the value of the transaction is greater then the balance, fail. + if (!sufficientBalance(txMeta.txParams, balance)) { + const message = 'Insufficient balance during rebroadcast.' + txMeta.warning = { + message, + } + this.emit('txWarning', txMeta) + log.error(message) + return + } + + // Only auto-submit already-signed txs: + if (!('rawTx' in txMeta)) return + + // Increment a try counter. + txMeta.retryCount++ + const rawTx = txMeta.rawTx + return await this.publishTransaction(rawTx) + } + + async _checkPendingTx (txMeta) { + const txHash = txMeta.hash + const txId = txMeta.id + // extra check in case there was an uncaught error during the + // signature and submission process + if (!txHash) { + const noTxHashErr = new Error('We had an error while submitting this transaction, please try again.') + noTxHashErr.name = 'NoTxHashError' + this.emit('txFailed', txId, noTxHashErr) + return + } + // get latest transaction status + let txParams + try { + txParams = await this.query.getTransactionByHash(txHash) + if (!txParams) return + if (txParams.blockNumber) { + this.emit('txConfirmed', txId) + } + } catch (err) { + txMeta.warning = { + error: err, + message: 'There was a problem loading this transaction.', + } + this.emit('txWarning', txMeta) + throw err + } + } + + // checks the network for signed txs and + // if confirmed sets the tx status as 'confirmed' + async _checkPendingTxs () { + const signedTxList = this.getPendingTransactions() + // in order to keep the nonceTracker accurate we block it while updating pending transactions + const nonceGlobalLock = await this.nonceTracker.getGlobalLock() + try { + await Promise.all(signedTxList.map((txMeta) => this._checkPendingTx(txMeta))) + } catch (err) { + console.error('PendingTransactionWatcher - Error updating pending transactions') + console.error(err) + } + nonceGlobalLock.releaseLock() + } +} \ No newline at end of file From cddff73703e9f6611e5ad793d6d05ba91ee96e06 Mon Sep 17 00:00:00 2001 From: frankiebee Date: Fri, 4 Aug 2017 14:42:13 -0400 Subject: [PATCH 15/38] bring your own BN --- package.json | 1 + 1 file changed, 1 insertion(+) diff --git a/package.json b/package.json index 9171bc206..6a2404337 100644 --- a/package.json +++ b/package.json @@ -53,6 +53,7 @@ "babel-runtime": "^6.23.0", "bip39": "^2.2.0", "bluebird": "^3.5.0", + "bn.js": "^4.11.7", "browser-passworder": "^2.0.3", "browserify-derequire": "^0.9.4", "client-sw-ready-event": "^3.3.0", From fa3df576bcf879904b303c4ed1015d27db64642d Mon Sep 17 00:00:00 2001 From: frankiebee Date: Fri, 4 Aug 2017 14:43:08 -0400 Subject: [PATCH 16/38] fixed: showing tx-s as errors vs. warning --- ui/app/components/transaction-list-item.js | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/ui/app/components/transaction-list-item.js b/ui/app/components/transaction-list-item.js index dbda66a31..01355abad 100644 --- a/ui/app/components/transaction-list-item.js +++ b/ui/app/components/transaction-list-item.js @@ -154,12 +154,21 @@ function failIfFailed (transaction) { if (transaction.status === 'rejected') { return h('span.error', ' (Rejected)') } - if (transaction.err) { + if (transaction.err || transaction.warning) { + const { err, warning = {} } = transaction + const errFirst = !!(( err && warning ) || err) + const message = errFirst ? err.message : warning.message + + errFirst ? err.message : warning.message + + return h(Tooltip, { - title: transaction.err.message, + title: message, position: 'bottom', }, [ - h('span.error', ' (Failed)'), + h(`span.${errFirst ? 'error' : 'warning'}`, + ` (${errFirst ? 'Failed' : 'Warning'})` + ), ]) } } From 08f49ab35f5a78fba6921a2957a92e0a2e5b065a Mon Sep 17 00:00:00 2001 From: frankiebee Date: Fri, 4 Aug 2017 14:50:34 -0400 Subject: [PATCH 17/38] rename PendingTransactionUtils -> PendingTransactionWatchers --- app/scripts/controllers/transactions.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index 498cac9af..c1a0ef0f3 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -5,7 +5,7 @@ const ObservableStore = require('obs-store') const ethUtil = require('ethereumjs-util') const EthQuery = require('ethjs-query') const TxProviderUtil = require('../lib/tx-utils') -const PendingTransactionUtils = require('../lib/pending-tx-watchers') +const PendingTransactionWatchers = require('../lib/pending-tx-watchers') const createId = require('../lib/random-id') const NonceTracker = require('../lib/nonce-tracker') @@ -37,7 +37,7 @@ module.exports = class TransactionController extends EventEmitter { this.query = new EthQuery(this.provider) this.txProviderUtils = new TxProviderUtil(this.provider) - this.pendingTxUtils = new PendingTransactionUtils({ + this.pendingTxWatcher = new PendingTransactionWatchers({ provider: this.provider, nonceTracker: this.nonceTracker, getBalance: (address) => this.ethStore.getState().accounts[address].balance, @@ -50,16 +50,16 @@ module.exports = class TransactionController extends EventEmitter { }, }) - this.pendingTxUtils.on('txWarning', this.updateTx.bind(this)) - this.pendingTxUtils.on('txFailed', this.setTxStatusFailed.bind(this)) - this.pendingTxUtils.on('txConfirmed', this.setTxStatusConfirmed.bind(this)) + this.pendingTxWatcher.on('txWarning', this.updateTx.bind(this)) + this.pendingTxWatcher.on('txFailed', this.setTxStatusFailed.bind(this)) + this.pendingTxWatcher.on('txConfirmed', this.setTxStatusConfirmed.bind(this)) - this.blockTracker.on('rawBlock', this.pendingTxUtils.checkForTxInBlock.bind(this)) + this.blockTracker.on('rawBlock', this.pendingTxWatcher.checkForTxInBlock.bind(this)) // this is a little messy but until ethstore has been either // removed or redone this is to guard against the race condition // where ethStore hasent been populated by the results yet - this.blockTracker.once('latest', () => this.blockTracker.on('latest', this.pendingTxUtils.resubmitPendingTxs.bind(this))) - this.blockTracker.on('sync', this.pendingTxUtils.queryPendingTxs.bind(this)) + this.blockTracker.once('latest', () => this.blockTracker.on('latest', this.pendingTxWatcher.resubmitPendingTxs.bind(this))) + this.blockTracker.on('sync', this.pendingTxWatcher.queryPendingTxs.bind(this)) // memstore is computed from a few different stores this._updateMemstore() this.store.subscribe(() => this._updateMemstore()) From fb9866b4e10c823e987d4cee9fc499673d664a8a Mon Sep 17 00:00:00 2001 From: frankiebee Date: Mon, 7 Aug 2017 14:37:20 -0400 Subject: [PATCH 18/38] fix spelling --- app/scripts/controllers/transactions.js | 18 +++++++++--------- app/scripts/lib/pending-tx-watchers.js | 9 +++------ 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index c1a0ef0f3..bc8e5b729 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -4,7 +4,7 @@ const clone = require('clone') const ObservableStore = require('obs-store') const ethUtil = require('ethereumjs-util') const EthQuery = require('ethjs-query') -const TxProviderUtil = require('../lib/tx-utils') +const TxProviderUtils = require('../lib/tx-utils') const PendingTransactionWatchers = require('../lib/pending-tx-watchers') const createId = require('../lib/random-id') const NonceTracker = require('../lib/nonce-tracker') @@ -35,9 +35,9 @@ module.exports = class TransactionController extends EventEmitter { }, }) this.query = new EthQuery(this.provider) - this.txProviderUtils = new TxProviderUtil(this.provider) + this.txProviderUtils = new TxProviderUtils(this.provider) - this.pendingTxWatcher = new PendingTransactionWatchers({ + this.pendingTxWatchers = new PendingTransactionWatchers({ provider: this.provider, nonceTracker: this.nonceTracker, getBalance: (address) => this.ethStore.getState().accounts[address].balance, @@ -50,16 +50,16 @@ module.exports = class TransactionController extends EventEmitter { }, }) - this.pendingTxWatcher.on('txWarning', this.updateTx.bind(this)) - this.pendingTxWatcher.on('txFailed', this.setTxStatusFailed.bind(this)) - this.pendingTxWatcher.on('txConfirmed', this.setTxStatusConfirmed.bind(this)) + this.pendingTxWatchers.on('txWarning', this.updateTx.bind(this)) + this.pendingTxWatchers.on('txFailed', this.setTxStatusFailed.bind(this)) + this.pendingTxWatchers.on('txConfirmed', this.setTxStatusConfirmed.bind(this)) - this.blockTracker.on('rawBlock', this.pendingTxWatcher.checkForTxInBlock.bind(this)) + this.blockTracker.on('rawBlock', this.pendingTxWatchers.checkForTxInBlock.bind(this)) // this is a little messy but until ethstore has been either // removed or redone this is to guard against the race condition // where ethStore hasent been populated by the results yet - this.blockTracker.once('latest', () => this.blockTracker.on('latest', this.pendingTxWatcher.resubmitPendingTxs.bind(this))) - this.blockTracker.on('sync', this.pendingTxWatcher.queryPendingTxs.bind(this)) + this.blockTracker.once('latest', () => this.blockTracker.on('latest', this.pendingTxWatchers.resubmitPendingTxs.bind(this))) + this.blockTracker.on('sync', this.pendingTxWatchers.queryPendingTxs.bind(this)) // memstore is computed from a few different stores this._updateMemstore() this.store.subscribe(() => this._updateMemstore()) diff --git a/app/scripts/lib/pending-tx-watchers.js b/app/scripts/lib/pending-tx-watchers.js index 4158e8bb5..5b23cc67c 100644 --- a/app/scripts/lib/pending-tx-watchers.js +++ b/app/scripts/lib/pending-tx-watchers.js @@ -19,7 +19,7 @@ const sufficientBalance = require('./util').sufficientBalance */ -module.exports = class PendingTransactionWatcher extends EventEmitter { +module.exports = class PendingTransactionWatchers extends EventEmitter { constructor (config) { super() this.query = new EthQuery(config.provider) @@ -101,11 +101,8 @@ module.exports = class PendingTransactionWatcher extends EventEmitter { // if the value of the transaction is greater then the balance, fail. if (!sufficientBalance(txMeta.txParams, balance)) { - const message = 'Insufficient balance during rebroadcast.' - txMeta.warning = { - message, - } - this.emit('txWarning', txMeta) + const insufficientFundsError = new Error('Insufficient balance during rebroadcast.') + this.emit('txFailed', txMeta.id, insufficientFundsError) log.error(message) return } From a54c26382e4e5d4e4fec543ac4e0ca90d0d91191 Mon Sep 17 00:00:00 2001 From: frankiebee Date: Mon, 7 Aug 2017 14:40:07 -0400 Subject: [PATCH 19/38] remove unnecessary if statment for error message --- app/scripts/controllers/transactions.js | 1 - 1 file changed, 1 deletion(-) diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index bc8e5b729..efc8d7117 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -246,7 +246,6 @@ module.exports = class TransactionController extends EventEmitter { // must set transaction to submitted/failed before releasing lock nonceLock.releaseLock() } catch (err) { - if (!err.message) err.message = 'Transaction failed during approval' this.setTxStatusFailed(txId, err) // must set transaction to submitted/failed before releasing lock if (nonceLock) nonceLock.releaseLock() From 59124eb6fd749cde1c021c69717d6f2c797428c7 Mon Sep 17 00:00:00 2001 From: frankiebee Date: Mon, 7 Aug 2017 14:45:43 -0400 Subject: [PATCH 20/38] remove logging of the message and log the error --- app/scripts/lib/pending-tx-watchers.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/scripts/lib/pending-tx-watchers.js b/app/scripts/lib/pending-tx-watchers.js index 5b23cc67c..60d0a09ad 100644 --- a/app/scripts/lib/pending-tx-watchers.js +++ b/app/scripts/lib/pending-tx-watchers.js @@ -103,7 +103,7 @@ module.exports = class PendingTransactionWatchers extends EventEmitter { if (!sufficientBalance(txMeta.txParams, balance)) { const insufficientFundsError = new Error('Insufficient balance during rebroadcast.') this.emit('txFailed', txMeta.id, insufficientFundsError) - log.error(message) + log.error(insufficientFundsError) return } From e7f838e626ff1ce29490a081d7ee3f917745dd62 Mon Sep 17 00:00:00 2001 From: frankiebee Date: Mon, 7 Aug 2017 14:56:17 -0400 Subject: [PATCH 21/38] fix test --- test/unit/pending-tx-test.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/unit/pending-tx-test.js b/test/unit/pending-tx-test.js index 5103a9b62..d0606bb17 100644 --- a/test/unit/pending-tx-test.js +++ b/test/unit/pending-tx-test.js @@ -224,7 +224,7 @@ describe('PendingTransactionWatcher', function () { pendingTxWatcher.resubmitPendingTxs() }) }) - describe('#_resubmitTx with a too-low balance', function () { + describe.only('#_resubmitTx with a too-low balance', function () { it('should return before publishing the transaction because to low of balance', function (done) { const lowBalance = '0x0' pendingTxWatcher.getBalance = (address) => { @@ -237,8 +237,8 @@ describe('PendingTransactionWatcher', function () { // Stubbing out current account state: // Adding the fake tx: - pendingTxWatcher.once('txWarning', (txMeta) => { - assert(txMeta.warning.message, 'Should have a warning message') + pendingTxWatcher.once('txFailed', (txId, err) => { + assert(err, 'Should have a error') done() }) pendingTxWatcher._resubmitTx(txMeta) From 28fbdca83006fa32c9411081f2152560005167ad Mon Sep 17 00:00:00 2001 From: frankiebee Date: Mon, 7 Aug 2017 16:41:54 -0400 Subject: [PATCH 22/38] remove .only --- test/unit/pending-tx-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/pending-tx-test.js b/test/unit/pending-tx-test.js index d0606bb17..6e50dae6a 100644 --- a/test/unit/pending-tx-test.js +++ b/test/unit/pending-tx-test.js @@ -224,7 +224,7 @@ describe('PendingTransactionWatcher', function () { pendingTxWatcher.resubmitPendingTxs() }) }) - describe.only('#_resubmitTx with a too-low balance', function () { + describe('#_resubmitTx with a too-low balance', function () { it('should return before publishing the transaction because to low of balance', function (done) { const lowBalance = '0x0' pendingTxWatcher.getBalance = (address) => { From 3a2190ec3cac0211d37dab6434d29f7bb484d4c4 Mon Sep 17 00:00:00 2001 From: frankiebee Date: Mon, 7 Aug 2017 16:57:17 -0400 Subject: [PATCH 23/38] fix the bind on pending tx watchers --- app/scripts/controllers/transactions.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index efc8d7117..cf987db91 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -54,12 +54,12 @@ module.exports = class TransactionController extends EventEmitter { this.pendingTxWatchers.on('txFailed', this.setTxStatusFailed.bind(this)) this.pendingTxWatchers.on('txConfirmed', this.setTxStatusConfirmed.bind(this)) - this.blockTracker.on('rawBlock', this.pendingTxWatchers.checkForTxInBlock.bind(this)) + this.blockTracker.on('rawBlock', this.pendingTxWatchers.checkForTxInBlock.bind(this.pendingTxWatchers)) // this is a little messy but until ethstore has been either // removed or redone this is to guard against the race condition // where ethStore hasent been populated by the results yet - this.blockTracker.once('latest', () => this.blockTracker.on('latest', this.pendingTxWatchers.resubmitPendingTxs.bind(this))) - this.blockTracker.on('sync', this.pendingTxWatchers.queryPendingTxs.bind(this)) + this.blockTracker.once('latest', () => this.blockTracker.on('latest', this.pendingTxWatchers.resubmitPendingTxs.bind(this.pendingTxWatchers))) + this.blockTracker.on('sync', this.pendingTxWatchers.queryPendingTxs.bind(this.pendingTxWatchers)) // memstore is computed from a few different stores this._updateMemstore() this.store.subscribe(() => this._updateMemstore()) From 67e4855b9d79afb3046c6499f4610cb5a8b70837 Mon Sep 17 00:00:00 2001 From: Thomas Huang Date: Tue, 8 Aug 2017 14:15:45 -0700 Subject: [PATCH 24/38] Updated commonjs require path for redux-logger and redux-thunk modules --- ui/app/store.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ui/app/store.js b/ui/app/store.js index ba9e58b49..3bafdee11 100644 --- a/ui/app/store.js +++ b/ui/app/store.js @@ -1,8 +1,8 @@ const createStore = require('redux').createStore const applyMiddleware = require('redux').applyMiddleware -const thunkMiddleware = require('redux-thunk') +const thunkMiddleware = require('redux-thunk').default const rootReducer = require('./reducers') -const createLogger = require('redux-logger') +const createLogger = require('redux-logger').createLogger global.METAMASK_DEBUG = 'GULP_METAMASK_DEBUG' From b6a7281737a320ab8a4d0819d5a8fce108f701dc Mon Sep 17 00:00:00 2001 From: sdtsui Date: Tue, 8 Aug 2017 15:17:24 -0700 Subject: [PATCH 25/38] Make buy ui more central to the screen --- ui/app/components/buy-button-subview.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/app/components/buy-button-subview.js b/ui/app/components/buy-button-subview.js index 87084f92d..759eb15bd 100644 --- a/ui/app/components/buy-button-subview.js +++ b/ui/app/components/buy-button-subview.js @@ -143,7 +143,7 @@ BuyButtonSubview.prototype.formVersionSubview = function () { return h('div.flex-column', { style: { alignItems: 'center', - margin: '50px', + margin: '20px 50px', }, }, [ h('h3.text-transform-uppercase', { From 9d3b5348aad0715f62c5bc2c0f4b5d0982ee2634 Mon Sep 17 00:00:00 2001 From: sdtsui Date: Tue, 8 Aug 2017 15:17:52 -0700 Subject: [PATCH 26/38] Fix layout issues flagged by @tmashuang --- ui/app/components/shapeshift-form.js | 148 ++++++++++++++------------- 1 file changed, 79 insertions(+), 69 deletions(-) diff --git a/ui/app/components/shapeshift-form.js b/ui/app/components/shapeshift-form.js index 76a265d63..901a4a956 100644 --- a/ui/app/components/shapeshift-form.js +++ b/ui/app/components/shapeshift-form.js @@ -33,10 +33,10 @@ ShapeshiftForm.prototype.renderMain = function () { return h('.flex-column', { style: { - // marginTop: '10px', + position: 'relative', padding: '25px', paddingTop: '5px', - width: '100%', + width: '90%', minHeight: '215px', alignItems: 'center', overflowY: 'auto', @@ -58,7 +58,9 @@ ShapeshiftForm.prototype.renderMain = function () { }, }), - h('.input-container', [ + h('.input-container', { + position: 'relative', + }, [ h('input#fromCoin.buy-inputs.ex-coins', { type: 'text', list: 'coinList', @@ -79,27 +81,31 @@ ShapeshiftForm.prototype.renderMain = function () { style: { fontSize: '12px', color: '#F7861C', - position: 'relative', - bottom: '48px', - left: '106px', + position: 'absolute', }, }), ]), - h('.icon-control', [ - h('i.fa.fa-refresh.fa-4.orange', { - style: { - bottom: '5px', - left: '5px', - color: '#F7861C', - }, - onClick: this.updateCoin.bind(this), - }), + h('.icon-control', { + style: { + position: 'relative', + }, + }, [ + // Not visible on the screen, can't see it on master. + + // h('i.fa.fa-refresh.fa-4.orange', { + // style: { + // bottom: '5px', + // left: '5px', + // color: '#F7861C', + // }, + // onClick: this.updateCoin.bind(this), + // }), h('i.fa.fa-chevron-right.fa-4.orange', { style: { - position: 'relative', - bottom: '26px', - left: '10px', + position: 'absolute', + bottom: '35%', + left: '0%', color: '#F7861C', }, onClick: this.updateCoin.bind(this), @@ -117,69 +123,73 @@ ShapeshiftForm.prototype.renderMain = function () { }, }), ]), + h('.flex-column', { style: { + marginTop: '1%', alignItems: 'flex-start', }, }, [ - this.props.warning ? this.props.warning && h('span.error.flex-center', { - style: { - textAlign: 'center', - width: '229px', - height: '82px', - }, - }, - this.props.warning) : this.renderInfo(), + this.props.warning + ? this.props.warning + && h('span.error.flex-center', { + style: { + textAlign: 'center', + width: '229px', + height: '82px', + }, + }, this.props.warning) + : this.renderInfo(), + + this.renderRefundAddressForCoin(coin), ]), - h(this.activeToggle('.input-container'), { + ]) +} + +ShapeshiftForm.prototype.renderRefundAddressForCoin = function (coin) { + return h(this.activeToggle('.input-container'), { + style: { + marginTop: '1%', + }, + }, [ + + h('div', `${coin} Address:`), + + h('input#fromCoinAddress.buy-inputs', { + type: 'text', + placeholder: `Your ${coin} Refund Address`, + dataset: { + persistentFormId: 'refund-address', + + }, style: { - padding: '10px', - paddingTop: '0px', - width: '100%', + boxSizing: 'border-box', + width: '227px', + height: '30px', + padding: ' 5px ', + }, + }), + + h('i.fa.fa-pencil-square-o.edit-text', { + style: { + fontSize: '12px', + color: '#F7861C', + position: 'absolute', + }, + }), + h('div.flex-row', { + style: { + justifyContent: 'flex-start', }, }, [ - - h('div', `${coin} Address:`), - - h('input#fromCoinAddress.buy-inputs', { - type: 'text', - placeholder: `Your ${coin} Refund Address`, - dataset: { - persistentFormId: 'refund-address', - }, + h('button', { + onClick: this.shift.bind(this), style: { - boxSizing: 'border-box', - width: '227px', - height: '30px', - padding: ' 5px ', + marginTop: '1%', }, - }), - - h('i.fa.fa-pencil-square-o.edit-text', { - style: { - fontSize: '12px', - color: '#F7861C', - position: 'relative', - bottom: '10px', - right: '11px', - }, - }), - h('.flex-row', { - style: { - justifyContent: 'flex-end', - }, - }, [ - h('button', { - onClick: this.shift.bind(this), - style: { - marginTop: '10px', - position: 'relative', - bottom: '40px', - }, - }, - 'Submit'), - ]), + }, + 'Submit'), ]), ]) } From be4011c310eab76d3010da2e6354a953396f85f5 Mon Sep 17 00:00:00 2001 From: frankiebee Date: Tue, 8 Aug 2017 18:30:03 -0400 Subject: [PATCH 27/38] create a provider stub --- test/stub/provider.js | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 test/stub/provider.js diff --git a/test/stub/provider.js b/test/stub/provider.js new file mode 100644 index 000000000..8a306f6d9 --- /dev/null +++ b/test/stub/provider.js @@ -0,0 +1,25 @@ +const JsonRpcEngine = require('json-rpc-engine') +const scaffoldMiddleware = require('eth-json-rpc-middleware/scaffold') + +module.exports = { + createEngineForTestData, + providerFromEngine, + scaffoldMiddleware, + createStubedProvider +} + + +function createEngineForTestData () { + return new JsonRpcEngine() +} + +function providerFromEngine (engine) { + const provider = { sendAsync: engine.handle.bind(engine) } + return provider +} + +function createStubedProvider (resultStub) { + const engine = createEngineForTestData() + engine.push(scaffoldMiddleware(resultStub)) + return providerFromEngine(engine) +} \ No newline at end of file From e761fb0ef7d7d658bd6e558fe2fcbe69d8eb4999 Mon Sep 17 00:00:00 2001 From: frankiebee Date: Tue, 8 Aug 2017 18:30:22 -0400 Subject: [PATCH 28/38] use provider stub --- test/unit/pending-tx-test.js | 119 +++++++++++++++----------------- test/unit/tx-controller-test.js | 34 +++------ 2 files changed, 65 insertions(+), 88 deletions(-) diff --git a/test/unit/pending-tx-test.js b/test/unit/pending-tx-test.js index 6e50dae6a..8c6d287f8 100644 --- a/test/unit/pending-tx-test.js +++ b/test/unit/pending-tx-test.js @@ -3,19 +3,20 @@ const ethUtil = require('ethereumjs-util') const EthTx = require('ethereumjs-tx') const ObservableStore = require('obs-store') const clone = require('clone') -const PendingTransactionWatcher = require('../../app/scripts/lib/pending-tx-watchers') +const { createStubedProvider } = require('../stub/provider') +const PendingTransactionTracker = require('../../app/scripts/lib/pending-tx-tracker') const noop = () => true const currentNetworkId = 42 const otherNetworkId = 36 const privKey = new Buffer('8718b9618a37d1fc78c436511fc6df3c8258d3250635bba617f33003270ec03e', 'hex') -describe('PendingTransactionWatcher', function () { - let pendingTxWatcher, txMeta, txMetaNoHash, txMetaNoRawTx +describe('PendingTransactionTracker', function () { + let pendingTxTracker, txMeta, txMetaNoHash, txMetaNoRawTx, providerResultStub, provider this.timeout(10000) beforeEach(function () { txMeta = { id: 1, - hash: '0x0593ee121b92e10d63150ad08b4b8f9c7857d1bd160195ee648fb9a0f8d00eebBlock', + hash: '0x0593ee121b92e10d63150ad08b4b8f9c7857d1bd160195ee648fb9a0f8d00eeb', status: 'signed', txParams: { from: '0x1678a085c290ebd122dc42cba69373b5953b831d', @@ -30,13 +31,15 @@ describe('PendingTransactionWatcher', function () { txParams: { from: '0x1678a085c290ebd122dc42cba69373b5953b831d'}, } txMetaNoRawTx = { - hash: '0x0593ee121b92e10d63150ad08b4b8f9c7857d1bd160195ee648fb9a0f8d00eebBlock', + hash: '0x0593ee121b92e10d63150ad08b4b8f9c7857d1bd160195ee648fb9a0f8d00eeb', status: 'signed', txParams: { from: '0x1678a085c290ebd122dc42cba69373b5953b831d'}, } + providerResultStub = {} + provider = createStubedProvider(providerResultStub) - pendingTxWatcher = new PendingTransactionWatcher({ - provider: { sendAsync: noop }, + pendingTxTracker = new PendingTransactionTracker({ + provider, getBalance: () => {}, nonceTracker: { getGlobalLock: async () => { @@ -47,20 +50,6 @@ describe('PendingTransactionWatcher', function () { sufficientBalance: () => {}, publishTransaction: () => {}, }) - - pendingTxWatcher.query = new Proxy({}, { - get: (queryStubResult, key) => { - if (key === 'stubResult') { - return function (method, ...args) { - queryStubResult[method] = args - } - } else { - const returnValues = queryStubResult[key] - return () => Promise.resolve(...returnValues) - } - }, - }) - }) describe('#checkForTxInBlock', function () { @@ -68,77 +57,77 @@ describe('PendingTransactionWatcher', function () { // throw a type error if it trys to do anything on the block // thus failing the test const block = Proxy.revocable({}, {}).revoke() - pendingTxWatcher.checkForTxInBlock(block) + pendingTxTracker.checkForTxInBlock(block) }) it('should emit \'txFailed\' if the txMeta does not have a hash', function (done) { const block = Proxy.revocable({}, {}).revoke() - pendingTxWatcher.getPendingTransactions = () => [txMetaNoHash] - pendingTxWatcher.once('txFailed', (txId, err) => { + pendingTxTracker.getPendingTransactions = () => [txMetaNoHash] + pendingTxTracker.once('txFailed', (txId, err) => { assert(txId, txMetaNoHash.id, 'should pass txId') done() }) - pendingTxWatcher.checkForTxInBlock(block) + pendingTxTracker.checkForTxInBlock(block) }) it('should emit \'txConfirmed\' if the tx is in the block', function (done) { const block = { transactions: [txMeta]} - pendingTxWatcher.getPendingTransactions = () => [txMeta] - pendingTxWatcher.once('txConfirmed', (txId) => { + pendingTxTracker.getPendingTransactions = () => [txMeta] + pendingTxTracker.once('txConfirmed', (txId) => { assert(txId, txMeta.id, 'should pass txId') done() }) - pendingTxWatcher.once('txFailed', (_, err) => { done(err) }) - pendingTxWatcher.checkForTxInBlock(block) + pendingTxTracker.once('txFailed', (_, err) => { done(err) }) + pendingTxTracker.checkForTxInBlock(block) }) }) describe('#queryPendingTxs', function () { it('should call #_checkPendingTxs if their is no oldBlock', function (done) { let newBlock, oldBlock newBlock = { number: '0x01' } - pendingTxWatcher._checkPendingTxs = done - pendingTxWatcher.queryPendingTxs({oldBlock, newBlock}) + pendingTxTracker._checkPendingTxs = done + pendingTxTracker.queryPendingTxs({oldBlock, newBlock}) }) it('should call #_checkPendingTxs if oldBlock and the newBlock have a diff of greater then 1', function (done) { let newBlock, oldBlock oldBlock = { number: '0x01' } newBlock = { number: '0x03' } - pendingTxWatcher._checkPendingTxs = done - pendingTxWatcher.queryPendingTxs({oldBlock, newBlock}) + pendingTxTracker._checkPendingTxs = done + pendingTxTracker.queryPendingTxs({oldBlock, newBlock}) }) it('should not call #_checkPendingTxs if oldBlock and the newBlock have a diff of 1 or less', function (done) { let newBlock, oldBlock oldBlock = { number: '0x1' } newBlock = { number: '0x2' } - pendingTxWatcher._checkPendingTxs = () => { + pendingTxTracker._checkPendingTxs = () => { const err = new Error('should not call #_checkPendingTxs if oldBlock and the newBlock have a diff of 1 or less') done(err) } - pendingTxWatcher.queryPendingTxs({oldBlock, newBlock}) + pendingTxTracker.queryPendingTxs({oldBlock, newBlock}) done() }) }) describe('#_checkPendingTx', function () { it('should emit \'txFailed\' if the txMeta does not have a hash', function (done) { - pendingTxWatcher.once('txFailed', (txId, err) => { + pendingTxTracker.once('txFailed', (txId, err) => { assert(txId, txMetaNoHash.id, 'should pass txId') done() }) - pendingTxWatcher._checkPendingTx(txMetaNoHash) + pendingTxTracker._checkPendingTx(txMetaNoHash) }) it('should should return if query does not return txParams', function () { - pendingTxWatcher.query.stubResult('getTransactionByHash', null) - pendingTxWatcher._checkPendingTx(txMeta) + providerResultStub.eth_getTransactionByHash = null + pendingTxTracker._checkPendingTx(txMeta) }) it('should emit \'txConfirmed\'', function (done) { - pendingTxWatcher.query.stubResult('getTransactionByHash', {blockNumber: '0x01'}) - pendingTxWatcher.once('txConfirmed', (txId) => { + providerResultStub.eth_getTransactionByHash = {blockNumber: '0x01'} + pendingTxTracker.once('txConfirmed', (txId) => { assert(txId, txMeta.id, 'should pass txId') done() }) - pendingTxWatcher.once('txFailed', (_, err) => { done(err) }) - pendingTxWatcher._checkPendingTx(txMeta) + pendingTxTracker.once('txFailed', (_, err) => { done(err) }) + pendingTxTracker._checkPendingTx(txMeta) }) }) @@ -154,14 +143,14 @@ describe('PendingTransactionWatcher', function () { }) it('should warp all txMeta\'s in #_checkPendingTx', function (done) { - pendingTxWatcher.getPendingTransactions = () => txList - pendingTxWatcher._checkPendingTx = (tx) => { tx.resolve(tx) } + pendingTxTracker.getPendingTransactions = () => txList + pendingTxTracker._checkPendingTx = (tx) => { tx.resolve(tx) } const list = txList.map Promise.all(txList.map((tx) => tx.processed)) .then((txCompletedList) => done()) .catch(done) - pendingTxWatcher._checkPendingTxs() + pendingTxTracker._checkPendingTxs() }) }) @@ -175,15 +164,15 @@ describe('PendingTransactionWatcher', function () { }) it('should return if no pending transactions', function () { - pendingTxWatcher.resubmitPendingTxs() + pendingTxTracker.resubmitPendingTxs() }) it('should call #_resubmitTx for all pending tx\'s', function (done) { - pendingTxWatcher.getPendingTransactions = () => txList - pendingTxWatcher._resubmitTx = async (tx) => { tx.resolve(tx) } + pendingTxTracker.getPendingTransactions = () => txList + pendingTxTracker._resubmitTx = async (tx) => { tx.resolve(tx) } Promise.all(txList.map((tx) => tx.processed)) .then((txCompletedList) => done()) .catch(done) - pendingTxWatcher.resubmitPendingTxs() + pendingTxTracker.resubmitPendingTxs() }) it('should not emit \'txFailed\' if the txMeta throws a known txError', function (done) { knownErrors =[ @@ -199,10 +188,10 @@ describe('PendingTransactionWatcher', function () { ] const enoughForAllErrors = txList.concat(txList) - pendingTxWatcher.on('txFailed', (_, err) => done(err)) + pendingTxTracker.on('txFailed', (_, err) => done(err)) - pendingTxWatcher.getPendingTransactions = () => enoughForAllErrors - pendingTxWatcher._resubmitTx = async (tx) => { + pendingTxTracker.getPendingTransactions = () => enoughForAllErrors + pendingTxTracker._resubmitTx = async (tx) => { tx.resolve() throw new Error(knownErrors.pop()) } @@ -210,38 +199,38 @@ describe('PendingTransactionWatcher', function () { .then((txCompletedList) => done()) .catch(done) - pendingTxWatcher.resubmitPendingTxs() + pendingTxTracker.resubmitPendingTxs() }) it('should emit \'txFailed\' if it encountered a real error', function (done) { - pendingTxWatcher.once('txFailed', (id, err) => err.message === 'im some real error' ? txList[id - 1].resolve() : done(err)) + pendingTxTracker.once('txFailed', (id, err) => err.message === 'im some real error' ? txList[id - 1].resolve() : done(err)) - pendingTxWatcher.getPendingTransactions = () => txList - pendingTxWatcher._resubmitTx = async (tx) => { throw new TypeError('im some real error') } + pendingTxTracker.getPendingTransactions = () => txList + pendingTxTracker._resubmitTx = async (tx) => { throw new TypeError('im some real error') } Promise.all(txList.map((tx) => tx.processed)) .then((txCompletedList) => done()) .catch(done) - pendingTxWatcher.resubmitPendingTxs() + pendingTxTracker.resubmitPendingTxs() }) }) describe('#_resubmitTx with a too-low balance', function () { it('should return before publishing the transaction because to low of balance', function (done) { const lowBalance = '0x0' - pendingTxWatcher.getBalance = (address) => { + pendingTxTracker.getBalance = (address) => { assert.equal(address, txMeta.txParams.from, 'Should pass the address') return lowBalance } - pendingTxWatcher.publishTransaction = async (rawTx) => { + pendingTxTracker.publishTransaction = async (rawTx) => { done(new Error('tried to publish transaction')) } // Stubbing out current account state: // Adding the fake tx: - pendingTxWatcher.once('txFailed', (txId, err) => { + pendingTxTracker.once('txFailed', (txId, err) => { assert(err, 'Should have a error') done() }) - pendingTxWatcher._resubmitTx(txMeta) + pendingTxTracker._resubmitTx(txMeta) .catch((err) => { assert.ifError(err, 'should not throw an error') done(err) @@ -250,17 +239,17 @@ describe('PendingTransactionWatcher', function () { it('should publishing the transaction', function (done) { const enoughBalance = '0x100000' - pendingTxWatcher.getBalance = (address) => { + pendingTxTracker.getBalance = (address) => { assert.equal(address, txMeta.txParams.from, 'Should pass the address') return enoughBalance } - pendingTxWatcher.publishTransaction = async (rawTx) => { + pendingTxTracker.publishTransaction = async (rawTx) => { assert.equal(rawTx, txMeta.rawTx, 'Should pass the rawTx') } // Stubbing out current account state: // Adding the fake tx: - pendingTxWatcher._resubmitTx(txMeta) + pendingTxTracker._resubmitTx(txMeta) .then(() => done()) .catch((err) => { assert.ifError(err, 'should not throw an error') diff --git a/test/unit/tx-controller-test.js b/test/unit/tx-controller-test.js index e54dc9719..57d7deccd 100644 --- a/test/unit/tx-controller-test.js +++ b/test/unit/tx-controller-test.js @@ -10,16 +10,20 @@ const noop = () => true const currentNetworkId = 42 const otherNetworkId = 36 const privKey = new Buffer('8718b9618a37d1fc78c436511fc6df3c8258d3250635bba617f33003270ec03e', 'hex') +const { createStubedProvider } = require('../stub/provider') describe('Transaction Controller', function () { - let txController + let txController, engine, provider, providerResultStub beforeEach(function () { + providerResultStub = {} + provider = createStubedProvider(providerResultStub) + txController = new TransactionController({ + provider, networkStore: new ObservableStore(currentNetworkId), txHistoryLimit: 10, blockTracker: { getCurrentBlock: noop, on: noop, once: noop }, - provider: { sendAsync: noop }, ethStore: { getState: noop }, signTransaction: (ethTx) => new Promise((resolve) => { ethTx.sign(privKey) @@ -28,19 +32,6 @@ describe('Transaction Controller', function () { }) txController.nonceTracker.getNonceLock = () => Promise.resolve({ nextNonce: 0, releaseLock: noop }) txController.txProviderUtils = new TxProvideUtils(txController.provider) - txController.query = txController.txProviderUtils.query = new Proxy({}, { - get: (queryStubResult, key) => { - if (key === 'stubResult') { - return function (method, ...args) { - queryStubResult[method] = args - } - } else { - const returnValues = queryStubResult[key] - return () => Promise.resolve(...returnValues) - } - }, - }) - }) describe('#newUnapprovedTransaction', function () { @@ -132,11 +123,9 @@ describe('Transaction Controller', function () { 'to':'0xc684832530fcbddae4b4230a47e991ddcec2831d', }, } - - txController.query.stubResult('gasPrice', '0x4a817c800') - txController.query.stubResult('getBlockByNumber', { gasLimit: '0x47b784' }) - txController.query.stubResult('estimateGas', '0x5209') - + providerResultStub.eth_gasPrice = '4a817c800' + providerResultStub.eth_getBlockByNumber = { gasLimit: '47b784' } + providerResultStub.eth_estimateGas = '5209' txController.addTxDefaults(txMeta) .then((txMetaWithDefaults) => { assert(txMetaWithDefaults.txParams.value, '0x0','should have added 0x0 as the value') @@ -393,9 +382,8 @@ describe('Transaction Controller', function () { const wrongValue = '0x05' txController.addTx(txMeta) - - txController.query.stubResult('estimateGas', wrongValue) - txController.query.stubResult('gasPrice', wrongValue) + providerResultStub.eth_gasPrice = wrongValue + providerResultStub.eth_estimateGas = '0x5209' const signStub = sinon.stub(txController, 'signTransaction').callsFake(() => Promise.resolve()) From a13643bdb545af60bd4514c9026e9657ce8aa5ea Mon Sep 17 00:00:00 2001 From: frankiebee Date: Tue, 8 Aug 2017 18:30:49 -0400 Subject: [PATCH 29/38] fix class names --- app/scripts/controllers/transactions.js | 38 +++++++++++-------- ...g-tx-watchers.js => pending-tx-tracker.js} | 3 +- app/scripts/lib/tx-utils.js | 2 +- 3 files changed, 25 insertions(+), 18 deletions(-) rename app/scripts/lib/{pending-tx-watchers.js => pending-tx-tracker.js} (98%) diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index cf987db91..664dbff6b 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -4,8 +4,8 @@ const clone = require('clone') const ObservableStore = require('obs-store') const ethUtil = require('ethereumjs-util') const EthQuery = require('ethjs-query') -const TxProviderUtils = require('../lib/tx-utils') -const PendingTransactionWatchers = require('../lib/pending-tx-watchers') +const TxProviderUtil = require('../lib/tx-utils') +const PendingTransactionTracker = require('../lib/pending-tx-tracker') const createId = require('../lib/random-id') const NonceTracker = require('../lib/nonce-tracker') @@ -35,13 +35,17 @@ module.exports = class TransactionController extends EventEmitter { }, }) this.query = new EthQuery(this.provider) - this.txProviderUtils = new TxProviderUtils(this.provider) + this.txProviderUtil = new TxProviderUtil(this.provider) - this.pendingTxWatchers = new PendingTransactionWatchers({ + this.pendingTxTracker = new PendingTransactionTracker({ provider: this.provider, nonceTracker: this.nonceTracker, - getBalance: (address) => this.ethStore.getState().accounts[address].balance, - publishTransaction: this.txProviderUtils.publishTransaction.bind(this.txProviderUtils), + getBalance: async (address) => { + const account = this.ethStore.getState().accounts[address] + if (!account) return + return account.balance + }, + publishTransaction: this.txProviderUtil.publishTransaction.bind(this.txProviderUtil), getPendingTransactions: (address) => { return this.getFilteredTxList({ from: address, @@ -50,16 +54,18 @@ module.exports = class TransactionController extends EventEmitter { }, }) - this.pendingTxWatchers.on('txWarning', this.updateTx.bind(this)) - this.pendingTxWatchers.on('txFailed', this.setTxStatusFailed.bind(this)) - this.pendingTxWatchers.on('txConfirmed', this.setTxStatusConfirmed.bind(this)) + this.pendingTxTracker.on('txWarning', this.updateTx.bind(this)) + this.pendingTxTracker.on('txFailed', this.setTxStatusFailed.bind(this)) + this.pendingTxTracker.on('txConfirmed', this.setTxStatusConfirmed.bind(this)) - this.blockTracker.on('rawBlock', this.pendingTxWatchers.checkForTxInBlock.bind(this.pendingTxWatchers)) + this.blockTracker.on('rawBlock', this.pendingTxTracker.checkForTxInBlock.bind(this.pendingTxTracker)) // this is a little messy but until ethstore has been either // removed or redone this is to guard against the race condition // where ethStore hasent been populated by the results yet - this.blockTracker.once('latest', () => this.blockTracker.on('latest', this.pendingTxWatchers.resubmitPendingTxs.bind(this.pendingTxWatchers))) - this.blockTracker.on('sync', this.pendingTxWatchers.queryPendingTxs.bind(this.pendingTxWatchers)) + this.blockTracker.once('latest', () => { + this.blockTracker.on('latest', this.pendingTxTracker.resubmitPendingTxs.bind(this.pendingTxTracker)) + }) + this.blockTracker.on('sync', this.pendingTxTracker.queryPendingTxs.bind(this.pendingTxTracker)) // memstore is computed from a few different stores this._updateMemstore() this.store.subscribe(() => this._updateMemstore()) @@ -191,7 +197,7 @@ module.exports = class TransactionController extends EventEmitter { async addUnapprovedTransaction (txParams) { // validate - await this.txProviderUtils.validateTxParams(txParams) + await this.txProviderUtil.validateTxParams(txParams) // construct txMeta const txMeta = { id: createId(), @@ -217,7 +223,7 @@ module.exports = class TransactionController extends EventEmitter { txParams.gasPrice = gasPrice } // set gasLimit - return await this.txProviderUtils.analyzeGasUsage(txMeta) + return await this.txProviderUtil.analyzeGasUsage(txMeta) } async updateAndApproveTransaction (txMeta) { @@ -260,7 +266,7 @@ module.exports = class TransactionController extends EventEmitter { const fromAddress = txParams.from // add network/chain id txParams.chainId = this.getChainId() - const ethTx = this.txProviderUtils.buildEthTxFromParams(txParams) + const ethTx = this.txProviderUtil.buildEthTxFromParams(txParams) await this.signEthTx(ethTx, fromAddress) this.setTxStatusSigned(txMeta.id) const rawTx = ethUtil.bufferToHex(ethTx.serialize()) @@ -271,7 +277,7 @@ module.exports = class TransactionController extends EventEmitter { const txMeta = this.getTx(txId) txMeta.rawTx = rawTx this.updateTx(txMeta) - const txHash = await this.txProviderUtils.publishTransaction(rawTx) + const txHash = await this.txProviderUtil.publishTransaction(rawTx) this.setTxHash(txId, txHash) this.setTxStatusSubmitted(txId) } diff --git a/app/scripts/lib/pending-tx-watchers.js b/app/scripts/lib/pending-tx-tracker.js similarity index 98% rename from app/scripts/lib/pending-tx-watchers.js rename to app/scripts/lib/pending-tx-tracker.js index 60d0a09ad..6921997b2 100644 --- a/app/scripts/lib/pending-tx-watchers.js +++ b/app/scripts/lib/pending-tx-tracker.js @@ -19,7 +19,7 @@ const sufficientBalance = require('./util').sufficientBalance */ -module.exports = class PendingTransactionWatchers extends EventEmitter { +module.exports = class PendingTransactionTracker extends EventEmitter { constructor (config) { super() this.query = new EthQuery(config.provider) @@ -97,6 +97,7 @@ module.exports = class PendingTransactionWatchers extends EventEmitter { async _resubmitTx (txMeta) { const address = txMeta.txParams.from const balance = this.getBalance(address) + if (balance === undefined) return if (!('retryCount' in txMeta)) txMeta.retryCount = 0 // if the value of the transaction is greater then the balance, fail. diff --git a/app/scripts/lib/tx-utils.js b/app/scripts/lib/tx-utils.js index a2db4abd8..b64ea6712 100644 --- a/app/scripts/lib/tx-utils.js +++ b/app/scripts/lib/tx-utils.js @@ -13,7 +13,7 @@ its passed ethquery and used to do things like calculate gas of a tx. */ -module.exports = class txProvideUtils { +module.exports = class txProvideUtil { constructor (provider) { this.query = new EthQuery(provider) } From 88b84e389513c3486e99c09b9a589c2a6f636248 Mon Sep 17 00:00:00 2001 From: frankiebee Date: Tue, 8 Aug 2017 18:34:59 -0400 Subject: [PATCH 30/38] add json-rpc-engine && eth-json-rpc-middleware to devDependencies --- package.json | 2 ++ 1 file changed, 2 insertions(+) diff --git a/package.json b/package.json index a275247ba..f35ac4963 100644 --- a/package.json +++ b/package.json @@ -155,6 +155,7 @@ "enzyme": "^2.8.2", "eslint-plugin-chai": "0.0.1", "eslint-plugin-mocha": "^4.9.0", + "eth-json-rpc-middleware": "^1.2.7", "fs-promise": "^1.0.0", "gulp": "github:gulpjs/gulp#4.0", "gulp-if": "^2.0.1", @@ -169,6 +170,7 @@ "jsdom": "^8.1.0", "jsdom-global": "^1.7.0", "jshint-stylish": "~0.1.5", + "json-rpc-engine": "^3.0.1", "lodash.assign": "^4.0.6", "mocha": "^2.4.5", "mocha-eslint": "^2.1.1", From 6767430aa2121e7b115ca2d2ca630a1d04ecd0e1 Mon Sep 17 00:00:00 2001 From: kumavis Date: Tue, 8 Aug 2017 17:46:09 -0700 Subject: [PATCH 31/38] deps - bump watchify --- package.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index 83c64cdab..d8f5c6167 100644 --- a/package.json +++ b/package.json @@ -155,8 +155,8 @@ "enzyme": "^2.8.2", "eslint-plugin-chai": "0.0.1", "eslint-plugin-mocha": "^4.9.0", - "fs-promise": "^2.0.3", "eth-json-rpc-middleware": "^1.2.7", + "fs-promise": "^2.0.3", "gulp": "github:gulpjs/gulp#4.0", "gulp-if": "^2.0.1", "gulp-json-editor": "^2.2.1", @@ -191,7 +191,7 @@ "uglifyify": "^4.0.2", "vinyl-buffer": "^1.0.0", "vinyl-source-stream": "^1.1.0", - "watchify": "^3.7.0" + "watchify": "^3.9.0" }, "engines": { "node": ">=0.8.0" From 4a4736195f258c44723a87c659f6380d5dd2cfdd Mon Sep 17 00:00:00 2001 From: kumavis Date: Tue, 8 Aug 2017 17:46:31 -0700 Subject: [PATCH 32/38] build - fix error swallowing on dev --- gulpfile.js | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/gulpfile.js b/gulpfile.js index cc723704a..57de37dd9 100644 --- a/gulpfile.js +++ b/gulpfile.js @@ -306,6 +306,17 @@ function bundleTask(opts) { return ( bundler.bundle() + + // handle errors + .on('error', (err) => { + beep() + if (opts.watch) { + console.warn(err.stack) + } else { + throw err + } + }) + // convert bundle stream to gulp vinyl stream .pipe(source(opts.filename)) // inject variables into bundle @@ -314,7 +325,7 @@ function bundleTask(opts) { .pipe(buffer()) // sourcemaps // loads map from browserify file - .pipe(gulpif(debug, sourcemaps.init({loadMaps: true}))) + .pipe(gulpif(debug, sourcemaps.init({ loadMaps: true }))) // writes .map file .pipe(gulpif(debug, sourcemaps.write('./'))) // write completed bundles @@ -328,3 +339,7 @@ function bundleTask(opts) { ) } } + +function beep () { + process.stdout.write('\x07') +} From 59e83d2b8063e7ef7c57c1fe9a2cd9c846637e20 Mon Sep 17 00:00:00 2001 From: kumavis Date: Tue, 8 Aug 2017 17:53:34 -0700 Subject: [PATCH 33/38] build - factor watcify into bundler generation --- gulpfile.js | 28 +++++++++++----------------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/gulpfile.js b/gulpfile.js index 57de37dd9..ac36cf983 100644 --- a/gulpfile.js +++ b/gulpfile.js @@ -248,26 +248,27 @@ function zipTask(target) { } } -function generateBundler(opts) { - var browserifyOpts = assign({}, watchify.args, { +function generateBundler(opts, performBundle) { + const browserifyOpts = assign({}, watchify.args, { entries: ['./app/scripts/'+opts.filename], plugin: 'browserify-derequire', debug: debug, fullPaths: debug, }) - return browserify(browserifyOpts) -} - -function discTask(opts) { - let bundler = generateBundler(opts) + let bundler = browserify(browserifyOpts) if (opts.watch) { bundler = watchify(bundler) - // on any dep update, runs the bundler + // on any file update, re-runs the bundler bundler.on('update', performBundle) } + return bundler +} + +function discTask(opts) { + const bundler = generateBundler(opts, performBundle) // output build logs to terminal bundler.on('log', gutil.log) @@ -289,14 +290,7 @@ function discTask(opts) { function bundleTask(opts) { - let bundler = generateBundler(opts) - - if (opts.watch) { - bundler = watchify(bundler) - // on any file update, re-runs the bundler - bundler.on('update', performBundle) - } - + const bundler = generateBundler(opts, performBundle) // output build logs to terminal bundler.on('log', gutil.log) @@ -316,7 +310,7 @@ function bundleTask(opts) { throw err } }) - + // convert bundle stream to gulp vinyl stream .pipe(source(opts.filename)) // inject variables into bundle From 5bb84f6e21842b9ac860be48c0ddc322d654559b Mon Sep 17 00:00:00 2001 From: frankiebee Date: Tue, 8 Aug 2017 21:49:48 -0400 Subject: [PATCH 34/38] fix getPendingTransactions function for pendingTxTracker --- app/scripts/controllers/transactions.js | 3 +-- app/scripts/lib/pending-tx-tracker.js | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index 664dbff6b..542881b8f 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -46,9 +46,8 @@ module.exports = class TransactionController extends EventEmitter { return account.balance }, publishTransaction: this.txProviderUtil.publishTransaction.bind(this.txProviderUtil), - getPendingTransactions: (address) => { + getPendingTransactions: () => { return this.getFilteredTxList({ - from: address, status: 'submitted', }) }, diff --git a/app/scripts/lib/pending-tx-tracker.js b/app/scripts/lib/pending-tx-tracker.js index 6921997b2..19720db3f 100644 --- a/app/scripts/lib/pending-tx-tracker.js +++ b/app/scripts/lib/pending-tx-tracker.js @@ -66,7 +66,7 @@ module.exports = class PendingTransactionTracker extends EventEmitter { resubmitPendingTxs () { - const pending = this.getPendingTransactions('status', 'submitted') + const pending = this.getPendingTransactions() // only try resubmitting if their are transactions to resubmit if (!pending.length) return pending.forEach((txMeta) => this._resubmitTx(txMeta).catch((err) => { From 9c9165e930684b53d525d6bced6df1a76030253b Mon Sep 17 00:00:00 2001 From: frankiebee Date: Tue, 8 Aug 2017 21:54:26 -0400 Subject: [PATCH 35/38] filter by network for pending txs --- app/scripts/controllers/transactions.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index 542881b8f..28130c164 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -47,8 +47,10 @@ module.exports = class TransactionController extends EventEmitter { }, publishTransaction: this.txProviderUtil.publishTransaction.bind(this.txProviderUtil), getPendingTransactions: () => { + const network = this.getNetwork() return this.getFilteredTxList({ status: 'submitted', + metamaskNetworkId: network, }) }, }) From 5418813ed146630cf2a84da2d9537955771e4d62 Mon Sep 17 00:00:00 2001 From: kumavis Date: Tue, 8 Aug 2017 21:05:59 -0700 Subject: [PATCH 36/38] util - sufficientBalance - validate input --- app/scripts/lib/util.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/app/scripts/lib/util.js b/app/scripts/lib/util.js index 70390e95c..6dee9edf0 100644 --- a/app/scripts/lib/util.js +++ b/app/scripts/lib/util.js @@ -1,5 +1,6 @@ const ethUtil = require('ethereumjs-util') -const BN = ethUtil.BN +const assert = require('assert') +const BN = require('bn.js') module.exports = { getStack, @@ -15,6 +16,10 @@ function getStack () { } function sufficientBalance (txParams, hexBalance) { + // validate hexBalance is a hex string + assert.equal(typeof hexBalance, 'string', 'sufficientBalance - hexBalance is not a hex string') + assert.equal(hexBalance.slice(0, 2), '0x', 'sufficientBalance - hexBalance is not a hex string') + const balance = hexToBn(hexBalance) const value = hexToBn(txParams.value) const gasLimit = hexToBn(txParams.gas) From 5e6962342df488073de61314fa4dfae9b7870fb8 Mon Sep 17 00:00:00 2001 From: kumavis Date: Tue, 8 Aug 2017 21:08:30 -0700 Subject: [PATCH 37/38] tx controller - fix getBalance fn --- app/scripts/controllers/transactions.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index 28130c164..8b5c6dde2 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -40,7 +40,7 @@ module.exports = class TransactionController extends EventEmitter { this.pendingTxTracker = new PendingTransactionTracker({ provider: this.provider, nonceTracker: this.nonceTracker, - getBalance: async (address) => { + getBalance: (address) => { const account = this.ethStore.getState().accounts[address] if (!account) return return account.balance From 25f9746dabb4ecc736a5cdd7b01b2d8a6c8151de Mon Sep 17 00:00:00 2001 From: kumavis Date: Tue, 8 Aug 2017 21:09:28 -0700 Subject: [PATCH 38/38] tx controller - fix error serialization --- app/scripts/controllers/transactions.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index 8b5c6dde2..58c468e22 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -382,9 +382,12 @@ module.exports = class TransactionController extends EventEmitter { this._setTxStatus(txId, 'confirmed') } - setTxStatusFailed (txId, reason) { + setTxStatusFailed (txId, err) { const txMeta = this.getTx(txId) - txMeta.err = reason + txMeta.err = { + message: err.toString(), + stack: err.stack, + } this.updateTx(txMeta) this._setTxStatus(txId, 'failed') }