From 51c5bebdf5ec4c4ac0e2878bd503e39a379d79c2 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Wed, 19 Jul 2017 11:44:00 -0700 Subject: [PATCH 01/53] Lowered minimum gas price to 1 gwei --- CHANGELOG.md | 1 + ui/app/components/pending-tx.js | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d265d318..e68a7ab89 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ - 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/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) From dcf025782b01265710a61a63da60d7d006df06b0 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Wed, 19 Jul 2017 11:56:53 -0700 Subject: [PATCH 02/53] Version 3.9.1 --- CHANGELOG.md | 2 ++ app/manifest.json | 2 +- package.json | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e68a7ab89..bf18bb361 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## Current Master +## 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. diff --git a/app/manifest.json b/app/manifest.json index 7bf757d4c..eadd99590 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.1", "manifest_version": 2, "author": "https://metamask.io", "description": "Ethereum Browser Extension", diff --git a/package.json b/package.json index d40ad068b..375902d09 100644 --- a/package.json +++ b/package.json @@ -67,7 +67,7 @@ "eth-contract-metadata": "^1.1.4", "eth-hd-keyring": "^1.1.1", "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", From 9018810ac20e7f73a34af564f208231196179f57 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Thu, 20 Jul 2017 09:55:10 -0700 Subject: [PATCH 03/53] Update tutorial links --- README.md | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 45fb68c78..bf1f37d1f 100644 --- a/README.md +++ b/README.md @@ -4,7 +4,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 From 366ddc62488c99494b3973e157bf9a1c9d5af0cb Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Thu, 20 Jul 2017 09:55:47 -0700 Subject: [PATCH 04/53] Add user support link --- README.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/README.md b/README.md index bf1f37d1f..d7086ae91 100644 --- a/README.md +++ b/README.md @@ -1,5 +1,9 @@ # MetaMask Plugin [![Build Status](https://circleci.com/gh/MetaMask/metamask-extension.svg?style=shield&circle-token=a1ddcf3cd38e29267f254c9c59d556d513e3a1fd)](https://circleci.com/gh/MetaMask/metamask-extension) +## Support + +If you're a user seeking support, [here is our support site](http://metamask.consensyssupport.happyfox.com). + ## Developing Compatible Dapps If you're a web dapp developer, we've got two types of guides for you: From d571f5ee701acd87495ad8ed69a74e6c5ca424f3 Mon Sep 17 00:00:00 2001 From: Thomas Huang Date: Sun, 23 Jul 2017 21:32:49 -0700 Subject: [PATCH 05/53] Add Test Coverage with nyc package and coveralls for github badge --- .gitignore | 5 ++++- README.md | 2 +- package.json | 3 +++ 3 files changed, 8 insertions(+), 2 deletions(-) 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/README.md b/README.md index d7086ae91..9aded09c1 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,4 @@ -# 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 diff --git a/package.json b/package.json index 375902d09..ef6e55980 100644 --- a/package.json +++ b/package.json @@ -13,6 +13,7 @@ "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 --reporter=text npm run test-unit", "lint": "gulp lint", "buildCiUnits": "node test/integration/index.js", "watch": "mocha watch --recursive \"test/unit/**/*.js\"", @@ -144,6 +145,7 @@ "brfs": "^1.4.3", "browserify": "^13.0.0", "chai": "^3.5.0", + "coveralls": "^2.13.1", "deep-freeze-strict": "^1.1.1", "del": "^2.2.0", "envify": "^4.0.0", @@ -170,6 +172,7 @@ "mocha-jsdom": "^1.1.0", "mocha-sinon": "^1.1.5", "nock": "^8.0.0", + "nyc": "^11.0.3", "open": "0.0.5", "prompt": "^1.0.0", "qs": "^6.2.0", From 55b7e457c504df0b0dadc848bc631704a950a14d Mon Sep 17 00:00:00 2001 From: Thomas Huang Date: Sun, 23 Jul 2017 21:35:21 -0700 Subject: [PATCH 06/53] Configure ci build to run tests individually --- circle.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/circle.yml b/circle.yml index 66eed17d7..efeb8ba57 100644 --- a/circle.yml +++ b/circle.yml @@ -5,3 +5,8 @@ dependencies: pre: - "npm i -g testem" - "npm i -g mocha" +test: + override: + - "npm run lint" + - "npm run test-coverage" + - "npm run test-integration" \ No newline at end of file From 24ffb40ec77016259ba4bb1b838298bf119f695e Mon Sep 17 00:00:00 2001 From: Thomas Huang Date: Mon, 24 Jul 2017 09:06:40 -0700 Subject: [PATCH 07/53] Add coveralls to script --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index ef6e55980..d90cc5e3b 100644 --- a/package.json +++ b/package.json @@ -13,7 +13,7 @@ "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 --reporter=text npm run test-unit", + "test-coverage": "nyc --reporter=lcov --reporter=text npm run test-unit && cat ./coverage/lcov.info | coveralls && rm -rf ./coverage", "lint": "gulp lint", "buildCiUnits": "node test/integration/index.js", "watch": "mocha watch --recursive \"test/unit/**/*.js\"", From a1fab0649035b75604dbee5aa18077c5cd747b3b Mon Sep 17 00:00:00 2001 From: Thomas Huang Date: Mon, 24 Jul 2017 13:46:02 -0700 Subject: [PATCH 08/53] Simplify the test-coverage script --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index d90cc5e3b..fe5466b9e 100644 --- a/package.json +++ b/package.json @@ -13,7 +13,7 @@ "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 --reporter=lcov --reporter=text npm run test-unit && cat ./coverage/lcov.info | coveralls && rm -rf ./coverage", + "test-coverage": "nyc npm run test-unit && nyc report --reporter=text-lcov | coveralls", "lint": "gulp lint", "buildCiUnits": "node test/integration/index.js", "watch": "mocha watch --recursive \"test/unit/**/*.js\"", From 77d91ec36f452ce47676b5ede2aa1b3d068d2634 Mon Sep 17 00:00:00 2001 From: kumavis Date: Tue, 25 Jul 2017 11:57:03 -0700 Subject: [PATCH 09/53] prov-eng - bump to ignore json parse errors --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 375902d09..9b582d3c9 100644 --- a/package.json +++ b/package.json @@ -127,7 +127,7 @@ "valid-url": "^1.0.9", "vreme": "^3.0.2", "web3": "0.19.1", - "web3-provider-engine": "^13.2.8", + "web3-provider-engine": "^13.2.9", "web3-stream-provider": "^3.0.1", "xtend": "^4.0.1" }, From ab01358a480243c9073ac06dc4f510a6089567d0 Mon Sep 17 00:00:00 2001 From: frankiebee Date: Tue, 25 Jul 2017 16:08:31 -0400 Subject: [PATCH 10/53] Add stack traces both in errors and as a way to track txMetas --- CHANGELOG.md | 2 ++ app/scripts/controllers/transactions.js | 30 ++++++++++++++++++------- app/scripts/lib/util.js | 8 +++++++ package.json | 1 + 4 files changed, 33 insertions(+), 8 deletions(-) create mode 100644 app/scripts/lib/util.js diff --git a/CHANGELOG.md b/CHANGELOG.md index bf18bb361..eeeda9d68 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## Current Master +- Include stack traces in txMeta's to better understand the life cycle of transactions + ## 3.9.1 2017-7-19 - No longer automatically request 1 ropsten ether for the first account in a new vault. diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index 5f3d84ebe..4d1a18df7 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -1,10 +1,12 @@ const EventEmitter = require('events') const async = require('async') const extend = require('xtend') +const clone = require('deep-clone') const ObservableStore = require('obs-store') const ethUtil = require('ethereumjs-util') const pify = require('pify') const TxProviderUtil = require('../lib/tx-utils') +const getStack = require('../lib/util').getStack const createId = require('../lib/random-id') const NonceTracker = require('../lib/nonce-tracker') @@ -117,9 +119,14 @@ module.exports = class TransactionController extends EventEmitter { // updateTx (txMeta) { + const txMetaForHistory = clone(txMeta) + txMetaForHistory.stack = getStack() var txId = txMeta.id var txList = this.getFullTxList() var 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') @@ -134,7 +141,7 @@ module.exports = class TransactionController extends EventEmitter { } addUnapprovedTransaction (txParams, done) { - let txMeta + let txMeta = {} async.waterfall([ // validate (cb) => this.txProviderUtils.validateTxParams(txParams, cb), @@ -146,6 +153,7 @@ module.exports = class TransactionController extends EventEmitter { status: 'unapproved', metamaskNetworkId: this.getNetwork(), txParams: txParams, + history: [], } cb() }, @@ -165,6 +173,7 @@ module.exports = class TransactionController extends EventEmitter { txParams.value = txParams.value || '0x0' if (!txParams.gasPrice) { this.query.gasPrice((err, gasPrice) => { + if (err) return cb(err) // set gasPrice txParams.gasPrice = gasPrice @@ -201,6 +210,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', }) @@ -364,11 +374,11 @@ module.exports = class TransactionController extends EventEmitter { var txId = txMeta.id if (!txHash) { - const errReason = { + return this.setTxStatusFailed(txId, { + stack: 'checkForTxInBlock: custom tx-controller error message Line# 368', errCode: 'No hash was provided', message: 'We had an error while submitting this transaction, please try again.', - } - return this.setTxStatusFailed(txId, errReason) + }) } block.transactions.forEach((tx) => { @@ -452,6 +462,7 @@ 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, }) @@ -466,7 +477,10 @@ 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 }) + this.setTxStatusFailed(txMeta.id, { + stack: '_resubnitTx: custom tx-controller error line# 472', + message, + }) cb() return log.error(message) } @@ -501,11 +515,11 @@ 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 = { + this.setTxStatusFailed(txId, { + stack: '_checkPendingTxs: custom tx-controller error message Line# 510', errCode: 'No hash was provided', message: 'We had an error while submitting this transaction, please try again.', - } - this.setTxStatusFailed(txId, errReason) + }) return } // get latest transaction status 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/package.json b/package.json index 375902d09..f18f84727 100644 --- a/package.json +++ b/package.json @@ -57,6 +57,7 @@ "clone": "^1.0.2", "copy-to-clipboard": "^2.0.0", "debounce": "^1.0.0", + "deep-clone": "^3.0.2", "deep-extend": "^0.4.1", "detect-node": "^2.0.3", "disc": "^1.3.2", From 5b9a6bd367173330d8bcfd973278eeba6f31ec06 Mon Sep 17 00:00:00 2001 From: kumavis Date: Tue, 25 Jul 2017 13:16:46 -0700 Subject: [PATCH 11/53] tx cont - remove old cb from async fn --- app/scripts/controllers/transactions.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index 5f3d84ebe..fc91bdf4d 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -458,7 +458,7 @@ module.exports = class TransactionController extends EventEmitter { })) } - async _resubmitTx (txMeta, cb) { + async _resubmitTx (txMeta) { const address = txMeta.txParams.from const balance = this.ethStore.getState().accounts[address].balance if (!('retryCount' in txMeta)) txMeta.retryCount = 0 @@ -467,17 +467,17 @@ module.exports = class TransactionController extends EventEmitter { if (!this.txProviderUtils.sufficientBalance(txMeta.txParams, balance)) { const message = 'Insufficient balance.' this.setTxStatusFailed(txMeta.id, { message }) - cb() - return log.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 From 4445ba15694fc6ef7f9c4e3a2de5de5428e28b3a Mon Sep 17 00:00:00 2001 From: kumavis Date: Tue, 25 Jul 2017 14:36:19 -0700 Subject: [PATCH 12/53] tx cont - add argument for provider constructor --- app/scripts/controllers/network.js | 5 ++-- test/unit/network-contoller-test.js | 36 +++++++++++++++++------------ 2 files changed, 24 insertions(+), 17 deletions(-) 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/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 From 5ec73c0e652f5389d14d00abed3977324043a824 Mon Sep 17 00:00:00 2001 From: kumavis Date: Tue, 25 Jul 2017 14:39:17 -0700 Subject: [PATCH 13/53] tx cont - fix test to use async api --- test/unit/tx-controller-test.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/test/unit/tx-controller-test.js b/test/unit/tx-controller-test.js index 7b86cfe14..31908569a 100644 --- a/test/unit/tx-controller-test.js +++ b/test/unit/tx-controller-test.js @@ -343,13 +343,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() + }) }) }) }) From e0a626da3b026fd7dd258e8accf771353c4ea38e Mon Sep 17 00:00:00 2001 From: frankiebee Date: Tue, 25 Jul 2017 18:02:21 -0400 Subject: [PATCH 14/53] remove line numbers --- 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 4d1a18df7..d96e2236a 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -375,7 +375,7 @@ module.exports = class TransactionController extends EventEmitter { if (!txHash) { return this.setTxStatusFailed(txId, { - stack: 'checkForTxInBlock: custom tx-controller error message Line# 368', + stack: 'checkForTxInBlock: custom tx-controller error me', errCode: 'No hash was provided', message: 'We had an error while submitting this transaction, please try again.', }) @@ -478,7 +478,7 @@ module.exports = class TransactionController extends EventEmitter { if (!this.txProviderUtils.sufficientBalance(txMeta.txParams, balance)) { const message = 'Insufficient balance.' this.setTxStatusFailed(txMeta.id, { - stack: '_resubnitTx: custom tx-controller error line# 472', + stack: '_resubnitTx: custom tx-controller ', message, }) cb() @@ -516,7 +516,7 @@ module.exports = class TransactionController extends EventEmitter { // signature and submission process if (!txHash) { this.setTxStatusFailed(txId, { - stack: '_checkPendingTxs: custom tx-controller error message Line# 510', + stack: '_checkPendingTxs: custom tx-controller error message', errCode: 'No hash was provided', message: 'We had an error while submitting this transaction, please try again.', }) From 1df833bee8a51c304491a3045138560e8c3f2b52 Mon Sep 17 00:00:00 2001 From: frankiebee Date: Tue, 25 Jul 2017 18:21:40 -0400 Subject: [PATCH 15/53] use clone --- app/scripts/controllers/transactions.js | 2 +- package.json | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index d96e2236a..4de2b7db3 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -1,7 +1,7 @@ const EventEmitter = require('events') const async = require('async') const extend = require('xtend') -const clone = require('deep-clone') +const clone = require('clone') const ObservableStore = require('obs-store') const ethUtil = require('ethereumjs-util') const pify = require('pify') diff --git a/package.json b/package.json index f18f84727..375902d09 100644 --- a/package.json +++ b/package.json @@ -57,7 +57,6 @@ "clone": "^1.0.2", "copy-to-clipboard": "^2.0.0", "debounce": "^1.0.0", - "deep-clone": "^3.0.2", "deep-extend": "^0.4.1", "detect-node": "^2.0.3", "disc": "^1.3.2", From b81f8831505b1ebb1d58a474d52b068d42879d56 Mon Sep 17 00:00:00 2001 From: frankiebee Date: Tue, 25 Jul 2017 18:23:17 -0400 Subject: [PATCH 16/53] fix stack wording --- app/scripts/controllers/transactions.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index 4de2b7db3..5485dc723 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -375,7 +375,7 @@ module.exports = class TransactionController extends EventEmitter { if (!txHash) { return this.setTxStatusFailed(txId, { - stack: 'checkForTxInBlock: custom tx-controller error me', + stack: 'checkForTxInBlock: custom tx-controller error message', errCode: 'No hash was provided', message: 'We had an error while submitting this transaction, please try again.', }) @@ -478,7 +478,7 @@ module.exports = class TransactionController extends EventEmitter { if (!this.txProviderUtils.sufficientBalance(txMeta.txParams, balance)) { const message = 'Insufficient balance.' this.setTxStatusFailed(txMeta.id, { - stack: '_resubnitTx: custom tx-controller ', + stack: '_resubnitTx: custom tx-controller error', message, }) cb() From ba88f7b8dd32b6ffdb46e70b8c9fbd563bb53b69 Mon Sep 17 00:00:00 2001 From: frankiebee Date: Tue, 25 Jul 2017 18:29:02 -0400 Subject: [PATCH 17/53] fix typo --- 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 5485dc723..263424518 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -478,7 +478,7 @@ module.exports = class TransactionController extends EventEmitter { if (!this.txProviderUtils.sufficientBalance(txMeta.txParams, balance)) { const message = 'Insufficient balance.' this.setTxStatusFailed(txMeta.id, { - stack: '_resubnitTx: custom tx-controller error', + stack: '_resubmitTx: custom tx-controller error', message, }) cb() From eb1566349723804e688f95a06f1208767e6d1938 Mon Sep 17 00:00:00 2001 From: Thomas Huang Date: Tue, 25 Jul 2017 16:33:52 -0700 Subject: [PATCH 18/53] One script runs for Ci build --- circle.yml | 4 +--- package.json | 1 + 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/circle.yml b/circle.yml index efeb8ba57..2ea60bb9d 100644 --- a/circle.yml +++ b/circle.yml @@ -7,6 +7,4 @@ dependencies: - "npm i -g mocha" test: override: - - "npm run lint" - - "npm run test-coverage" - - "npm run test-integration" \ No newline at end of file + - "npm run ci" \ No newline at end of file diff --git a/package.json b/package.json index fe5466b9e..6d5227356 100644 --- a/package.json +++ b/package.json @@ -14,6 +14,7 @@ "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\"", From 3575bd49ab4f3ec879a3e04b4a6fa35fe9c67c6e Mon Sep 17 00:00:00 2001 From: Thomas Huang Date: Tue, 25 Jul 2017 16:59:08 -0700 Subject: [PATCH 19/53] Run coveralls on all branches --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 9aded09c1..27b19c91c 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,4 @@ -# 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) +# 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) ## Support From 1eb089b0e76336719bf4c627755a7aa789280808 Mon Sep 17 00:00:00 2001 From: Thomas Huang Date: Tue, 25 Jul 2017 18:03:47 -0700 Subject: [PATCH 20/53] Coveralls badge directs to master branch --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 27b19c91c..9aded09c1 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,4 @@ -# 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) +# 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 From f16802e2d4f7e917e894e3ec38a716255f6b0942 Mon Sep 17 00:00:00 2001 From: kumavis Date: Wed, 26 Jul 2017 10:15:35 -0700 Subject: [PATCH 21/53] nonce-tracker - validation - add validation failing value to error message --- app/scripts/lib/nonce-tracker.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/scripts/lib/nonce-tracker.js b/app/scripts/lib/nonce-tracker.js index b76dac4e8..4bba1f1a8 100644 --- a/app/scripts/lib/nonce-tracker.js +++ b/app/scripts/lib/nonce-tracker.js @@ -31,12 +31,12 @@ 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: "${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: "${baseCount}"`) const nextNonce = baseCount + pendingCount - assert(Number.isInteger(nextNonce), 'nonce-tracker - nextNonce is an integer') + assert(Number.isInteger(nextNonce), `nonce-tracker - nextNonce is not an integer - got: "${nextNonce}"`) // return next nonce and release cb return { nextNonce, releaseLock } } From 39d28922de31fa26b50eca5c7719ae9feefae770 Mon Sep 17 00:00:00 2001 From: kumavis Date: Wed, 26 Jul 2017 10:16:08 -0700 Subject: [PATCH 22/53] nonce-tracker - validation - add validation failing value type to error message --- app/scripts/lib/nonce-tracker.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/scripts/lib/nonce-tracker.js b/app/scripts/lib/nonce-tracker.js index 4bba1f1a8..81b500550 100644 --- a/app/scripts/lib/nonce-tracker.js +++ b/app/scripts/lib/nonce-tracker.js @@ -31,12 +31,12 @@ class NonceTracker { const currentBlock = await this._getCurrentBlock() const pendingTransactions = this.getPendingTransactions(address) const pendingCount = pendingTransactions.length - assert(Number.isInteger(pendingCount), `nonce-tracker - pendingCount is not an integer - got: "${pendingCount}"`) + 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 not an integer - got: "${baseCount}"`) + 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 not an integer - got: "${nextNonce}"`) + assert(Number.isInteger(nextNonce), `nonce-tracker - nextNonce is not an integer - got: (${typeof nextNonce}) "${nextNonce}"`) // return next nonce and release cb return { nextNonce, releaseLock } } From 0ef90fb1f0f1a1bf4a7efd90df7b8f8c66fc07d5 Mon Sep 17 00:00:00 2001 From: kumavis Date: Wed, 26 Jul 2017 10:40:08 -0700 Subject: [PATCH 23/53] tx controller + nonce tracker - record nonce components on txMeta --- app/scripts/controllers/transactions.js | 4 ++++ app/scripts/lib/nonce-tracker.js | 7 +++++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index 7b2e4e314..32795a9f2 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -200,8 +200,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) diff --git a/app/scripts/lib/nonce-tracker.js b/app/scripts/lib/nonce-tracker.js index 81b500550..c0746bd87 100644 --- a/app/scripts/lib/nonce-tracker.js +++ b/app/scripts/lib/nonce-tracker.js @@ -37,8 +37,11 @@ class NonceTracker { 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 not an integer - got: (${typeof nextNonce}) "${nextNonce}"`) - // return next nonce and release cb - return { nextNonce, releaseLock } + // collect the numbers used to calculate the nonce for debugging + const blockNumber = currentBlock.number + const nonceDetails = { blockNumber, baseCount, pendingCount } + // return nonce and release cb + return { nextNonce, nonceDetails, releaseLock } } async _getCurrentBlock () { From 9d69951decc0cfb0e3dc8bd40bd98f4d1c524dc1 Mon Sep 17 00:00:00 2001 From: kumavis Date: Wed, 26 Jul 2017 10:50:06 -0700 Subject: [PATCH 24/53] logState - dont redundantly log to console --- ui/app/reducers.js | 1 - 1 file changed, 1 deletion(-) 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 7e2e4948a6ce5856338406de49cbad6a9931d72b Mon Sep 17 00:00:00 2001 From: kumavis Date: Wed, 26 Jul 2017 10:57:47 -0700 Subject: [PATCH 25/53] tx cont - dont recursively store history --- app/scripts/controllers/transactions.js | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index d6b2b555e..8f53ffa8c 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -119,14 +119,20 @@ module.exports = class TransactionController extends EventEmitter { // updateTx (txMeta) { + // create txMeta snapshot for history const txMetaForHistory = clone(txMeta) + // dont include previous history in this snapshot + delete txMetaForHistory.history + // add stack to help understand why tx was updated txMetaForHistory.stack = getStack() - var txId = txMeta.id - var txList = this.getFullTxList() - var index = txList.findIndex(txData => txData.id === txId) + // add snapshot to tx history if (!txMeta.history) txMeta.history = [] txMeta.history.push(txMetaForHistory) + // update the tx + 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') From b15a2baaf3cf7b4850c427857e935b238d1e5cc2 Mon Sep 17 00:00:00 2001 From: kumavis Date: Wed, 26 Jul 2017 11:09:02 -0700 Subject: [PATCH 26/53] nonce-tracker - add raw baseNonceHex to nonceDetails --- app/scripts/lib/nonce-tracker.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/scripts/lib/nonce-tracker.js b/app/scripts/lib/nonce-tracker.js index c0746bd87..e33073ac1 100644 --- a/app/scripts/lib/nonce-tracker.js +++ b/app/scripts/lib/nonce-tracker.js @@ -39,7 +39,7 @@ class NonceTracker { 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, pendingCount } + const nonceDetails = { blockNumber, baseCount, baseCountHex, pendingCount } // return nonce and release cb return { nextNonce, nonceDetails, releaseLock } } From 35a128db1e6ecba9076ec145c9d2334f623703b7 Mon Sep 17 00:00:00 2001 From: kumavis Date: Wed, 26 Jul 2017 11:37:00 -0700 Subject: [PATCH 27/53] nonce-tracker - hotfix for provider proxying --- app/scripts/controllers/transactions.js | 1 - app/scripts/lib/nonce-tracker.js | 15 +++++++++++---- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index 8f53ffa8c..f71659042 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -24,7 +24,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, diff --git a/app/scripts/lib/nonce-tracker.js b/app/scripts/lib/nonce-tracker.js index e33073ac1..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 = {} @@ -45,10 +45,11 @@ class NonceTracker { } 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) }) } @@ -82,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 From 55a55141d08f7827cb23f4213a6d88351803fbff Mon Sep 17 00:00:00 2001 From: kumavis Date: Wed, 26 Jul 2017 11:43:37 -0700 Subject: [PATCH 28/53] nonce-tracker - fix test --- test/unit/nonce-tracker-test.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) 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, }) From 3d8ebf2265d167923f3b913bac3b9cc4d37fa052 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Wed, 26 Jul 2017 12:10:42 -0700 Subject: [PATCH 29/53] Begin implementing live-updating blacklist, not working yet --- app/scripts/background.js | 27 +++++++++++++++++++++++++++ app/scripts/blacklister.js | 19 +++++++++++-------- app/scripts/controllers/infura.js | 14 ++++++++++++++ 3 files changed, 52 insertions(+), 8 deletions(-) diff --git a/app/scripts/background.js b/app/scripts/background.js index e8987394f..c9505b237 100644 --- a/app/scripts/background.js +++ b/app/scripts/background.js @@ -90,6 +90,10 @@ function setupController (initState) { extension.runtime.onConnect.addListener(connectRemote) function connectRemote (remotePort) { + if (remotePort.name === 'blacklister') { + return setupBlacklist(connectRemote) + } + var isMetaMaskInternalProcess = remotePort.name === 'popup' || remotePort.name === 'notification' var portStream = new PortStream(remotePort) if (isMetaMaskInternalProcess) { @@ -135,6 +139,29 @@ function setupController (initState) { return Promise.resolve() } +// Listen for new pages and return if blacklisted: +function setupBlacklist (port) { + console.log('Blacklist connection established') + const handler = handleNewPageLoad.bind(port) + port.onMessage.addListener(handler) + setTimeout(() => { + port.onMessage.removeListener(handler) + }, 30000) +} + +function handleNewPageLoad (message) { + const { pageLoaded } = message + console.log('blaclist message received', message.pageLoaded) + if (!pageLoaded || !global.metamaskController) return + + const state = global.metamaskController.getState() + const { blacklist } = state.metamask + + if (blacklist && blacklist.includes(pageLoaded)) { + this.postMessage({ 'blacklist': pageLoaded }) + } +} + // // Etc... // diff --git a/app/scripts/blacklister.js b/app/scripts/blacklister.js index a45265a75..f5572c11a 100644 --- a/app/scripts/blacklister.js +++ b/app/scripts/blacklister.js @@ -1,13 +1,16 @@ -const blacklistedDomains = require('etheraddresslookup/blacklists/domains.json') +const extension = require('extensionizer') +console.log('blacklister content script loaded.') -function detectBlacklistedDomain() { - var strCurrentTab = window.location.hostname - if (blacklistedDomains && blacklistedDomains.includes(strCurrentTab)) { +const port = extension.runtime.connect({ name: 'blacklister' }) +port.postMessage({ 'pageLoaded': window.location.hostname }) +port.onMessage.addListener(redirectIfBlacklisted) + +function redirectIfBlacklisted (response) { + const { blacklist } = response + console.log('blacklister contentscript received blacklist response') + const host = window.location.hostname + if (blacklist && blacklist === host) { window.location.href = 'https://metamask.io/phishing.html' } } -window.addEventListener('load', function() { - detectBlacklistedDomain() -}) - diff --git a/app/scripts/controllers/infura.js b/app/scripts/controllers/infura.js index b34b0bc03..97b2ab7e3 100644 --- a/app/scripts/controllers/infura.js +++ b/app/scripts/controllers/infura.js @@ -1,5 +1,6 @@ const ObservableStore = require('obs-store') const extend = require('xtend') +const recentBlacklist = require('etheraddresslookup/blacklists/domains.json') // every ten minutes const POLLING_INTERVAL = 300000 @@ -9,6 +10,7 @@ class InfuraController { constructor (opts = {}) { const initState = extend({ infuraNetworkStatus: {}, + blacklist: recentBlacklist, }, opts.initState) this.store = new ObservableStore(initState) } @@ -30,12 +32,24 @@ class InfuraController { }) } + updateLocalBlacklist () { + return fetch('https://api.infura.io/v1/blacklist') + .then(response => response.json()) + .then((parsedResponse) => { + this.store.updateState({ + blacklist: parsedResponse, + }) + return parsedResponse + }) + } + scheduleInfuraNetworkCheck () { if (this.conversionInterval) { clearInterval(this.conversionInterval) } this.conversionInterval = setInterval(() => { this.checkInfuraNetworkStatus() + this.updateLocalBlacklist() }, POLLING_INTERVAL) } } From b50c10f373c30642e083fb87724fc5db2ffff1e9 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Wed, 26 Jul 2017 14:15:24 -0700 Subject: [PATCH 30/53] Version 3.9.2 --- CHANGELOG.md | 3 +++ app/manifest.json | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eeeda9d68..c5f727586 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,9 @@ ## Current Master +## 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 ## 3.9.1 2017-7-19 diff --git a/app/manifest.json b/app/manifest.json index eadd99590..55e1eb5b1 100644 --- a/app/manifest.json +++ b/app/manifest.json @@ -1,7 +1,7 @@ { "name": "MetaMask", "short_name": "Metamask", - "version": "3.9.1", + "version": "3.9.2", "manifest_version": 2, "author": "https://metamask.io", "description": "Ethereum Browser Extension", From 6a9d40c558564763ee06deb4861238b1b06e1f00 Mon Sep 17 00:00:00 2001 From: Kevin Serrano Date: Wed, 26 Jul 2017 15:24:57 -0700 Subject: [PATCH 31/53] Add test for blacklister. --- test/unit/blacklister-test.js | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100644 test/unit/blacklister-test.js diff --git a/test/unit/blacklister-test.js b/test/unit/blacklister-test.js new file mode 100644 index 000000000..d9290795c --- /dev/null +++ b/test/unit/blacklister-test.js @@ -0,0 +1,24 @@ +const assert = require('assert') +const Blacklister = require('../../app/scripts/blacklister') + + +describe('blacklister', function () { + describe('#isPhish', function () { + it('should not flag whitelisted values', function () { + var result = Blacklister('www.metamask.io') + assert(!result) + }) + it('should flag explicit values', function () { + var result = Blacklister('metamask.com') + assert(result) + }) + it('should flag levenshtein values', function () { + var result = Blacklister('metmask.io') + assert(result) + }) + it('should not flag not-even-close values', function () { + var result = Blacklister('example.com') + assert(!result) + }) + }) +}) From 66f6d5a4e06c6938ae22bd2cb4696f6ade900df2 Mon Sep 17 00:00:00 2001 From: Kevin Serrano Date: Wed, 26 Jul 2017 15:25:30 -0700 Subject: [PATCH 32/53] Add levenshtein logic to blacklister. --- app/scripts/blacklister.js | 40 ++++++++++++++++++++++++++++++-------- package.json | 1 + 2 files changed, 33 insertions(+), 8 deletions(-) diff --git a/app/scripts/blacklister.js b/app/scripts/blacklister.js index a45265a75..f4b95a31f 100644 --- a/app/scripts/blacklister.js +++ b/app/scripts/blacklister.js @@ -1,13 +1,37 @@ -const blacklistedDomains = require('etheraddresslookup/blacklists/domains.json') +const levenshtein = require('fast-levenshtein') +const blacklistedMetaMaskDomains = ['metamask.com'] +const blacklistedDomains = require('etheraddresslookup/blacklists/domains.json').concat(blacklistedMetaMaskDomains) +const whitelistedMetaMaskDomains = ['metamask.io', 'www.metamask.io'] +const whitelistedDomains = require('etheraddresslookup/whitelists/domains.json').concat(whitelistedMetaMaskDomains) +const LEVENSHTEIN_TOLERANCE = 4 +const LEVENSHTEIN_CHECKS = ['myetherwallet', 'myetheroll', 'ledgerwallet', 'metamask'] -function detectBlacklistedDomain() { - var strCurrentTab = window.location.hostname - if (blacklistedDomains && blacklistedDomains.includes(strCurrentTab)) { - window.location.href = 'https://metamask.io/phishing.html' - } +function isPhish(hostname) { + var strCurrentTab = hostname + + // check if the domain is part of the whitelist. + if (whitelistedDomains && whitelistedDomains.includes(strCurrentTab)) { return false } + + // check if the domain is part of the blacklist. + var isBlacklisted = blacklistedDomains && blacklistedDomains.includes(strCurrentTab) + + // check for similar values. + var levenshteinMatched = false + var levenshteinForm = strCurrentTab.replace(/\./g, '') + LEVENSHTEIN_CHECKS.forEach((element) => { + if (levenshtein.get(element, levenshteinForm) < LEVENSHTEIN_TOLERANCE) { + levenshteinMatched = true + } + }) + + return isBlacklisted || levenshteinMatched } -window.addEventListener('load', function() { - detectBlacklistedDomain() +window.addEventListener('load', function () { + var hostnameToCheck = window.location.hostname + if (isPhish(hostnameToCheck)) { + window.location.href = 'https://metamask.io/phishing.html' + } }) +module.exports = isPhish diff --git a/package.json b/package.json index dcd25cda6..10afc8228 100644 --- a/package.json +++ b/package.json @@ -80,6 +80,7 @@ "express": "^4.14.0", "extension-link-enabler": "^1.0.0", "extensionizer": "^1.0.0", + "fast-levenshtein": "^2.0.6", "gulp-eslint": "^2.0.0", "hat": "0.0.3", "idb-global": "^1.0.0", From a6395436653f274b39bbf642a4d502879bd6eb92 Mon Sep 17 00:00:00 2001 From: Kevin Serrano Date: Wed, 26 Jul 2017 15:29:13 -0700 Subject: [PATCH 33/53] Changelog bump --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index eeeda9d68..46de6fd97 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## Current Master - Include stack traces in txMeta's to better understand the life cycle of transactions +- Enhance blacklister functionality to include levenshtein logic. ## 3.9.1 2017-7-19 From aa282b4e3a55d090f27e37cacf850aa5298cfe27 Mon Sep 17 00:00:00 2001 From: Kevin Serrano Date: Wed, 26 Jul 2017 15:31:16 -0700 Subject: [PATCH 34/53] Give credit where it is due --- CHANGELOG.md | 2 +- app/scripts/blacklister.js | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 46de6fd97..bb57870fb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,7 +3,7 @@ ## Current Master - Include stack traces in txMeta's to better understand the life cycle of transactions -- Enhance blacklister functionality to include levenshtein logic. +- Enhance blacklister functionality to include levenshtein logic. (credit to @sogoiii and @409H for their help!) ## 3.9.1 2017-7-19 diff --git a/app/scripts/blacklister.js b/app/scripts/blacklister.js index f4b95a31f..9337599cc 100644 --- a/app/scripts/blacklister.js +++ b/app/scripts/blacklister.js @@ -6,6 +6,9 @@ const whitelistedDomains = require('etheraddresslookup/whitelists/domains.json') const LEVENSHTEIN_TOLERANCE = 4 const LEVENSHTEIN_CHECKS = ['myetherwallet', 'myetheroll', 'ledgerwallet', 'metamask'] + +// credit to @sogoiii and @409H for their help! +// Return a boolean on whether or not a phish is detected. function isPhish(hostname) { var strCurrentTab = hostname @@ -30,6 +33,7 @@ function isPhish(hostname) { window.addEventListener('load', function () { var hostnameToCheck = window.location.hostname if (isPhish(hostnameToCheck)) { + // redirect to our phishing warning page. window.location.href = 'https://metamask.io/phishing.html' } }) From 8b1726cc550d4a5b142a2a525ce6b94713dc04e0 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Wed, 26 Jul 2017 16:30:54 -0700 Subject: [PATCH 35/53] Live update blacklist from Infura --- CHANGELOG.md | 2 ++ app/manifest.json | 9 +++++++-- app/scripts/background.js | 14 ++++++-------- app/scripts/blacklister.js | 4 +--- app/scripts/inpage.js | 1 + 5 files changed, 17 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c5f727586..ba8bdd16c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## Current Master +- Continuously update blacklist for known phishing sites in background. + ## 3.9.2 2017-7-26 - Fix bugs that could sometimes result in failed transactions after switching networks. diff --git a/app/manifest.json b/app/manifest.json index 55e1eb5b1..edc4d7162 100644 --- a/app/manifest.json +++ b/app/manifest.json @@ -55,8 +55,13 @@ }, { "run_at": "document_start", - "matches": ["http://*/*", "https://*/*"], - "js": ["scripts/blacklister.js"] + "matches": [ + "file://*/*", + "http://*/*", + "https://*/*" + ], + "js": ["scripts/blacklister.js"], + "all_frames": true } ], "permissions": [ diff --git a/app/scripts/background.js b/app/scripts/background.js index c9505b237..01bb39186 100644 --- a/app/scripts/background.js +++ b/app/scripts/background.js @@ -91,7 +91,7 @@ function setupController (initState) { extension.runtime.onConnect.addListener(connectRemote) function connectRemote (remotePort) { if (remotePort.name === 'blacklister') { - return setupBlacklist(connectRemote) + return checkBlacklist(remotePort) } var isMetaMaskInternalProcess = remotePort.name === 'popup' || remotePort.name === 'notification' @@ -140,25 +140,23 @@ function setupController (initState) { } // Listen for new pages and return if blacklisted: -function setupBlacklist (port) { - console.log('Blacklist connection established') - const handler = handleNewPageLoad.bind(port) +function checkBlacklist (port) { + const handler = handleNewPageLoad.bind(null, port) port.onMessage.addListener(handler) setTimeout(() => { port.onMessage.removeListener(handler) }, 30000) } -function handleNewPageLoad (message) { +function handleNewPageLoad (port, message) { const { pageLoaded } = message - console.log('blaclist message received', message.pageLoaded) if (!pageLoaded || !global.metamaskController) return const state = global.metamaskController.getState() - const { blacklist } = state.metamask + const { blacklist } = state if (blacklist && blacklist.includes(pageLoaded)) { - this.postMessage({ 'blacklist': pageLoaded }) + port.postMessage({ 'blacklist': pageLoaded }) } } diff --git a/app/scripts/blacklister.js b/app/scripts/blacklister.js index f5572c11a..37751b595 100644 --- a/app/scripts/blacklister.js +++ b/app/scripts/blacklister.js @@ -1,13 +1,11 @@ const extension = require('extensionizer') -console.log('blacklister content script loaded.') -const port = extension.runtime.connect({ name: 'blacklister' }) +var port = extension.runtime.connect({name: 'blacklister'}) port.postMessage({ 'pageLoaded': window.location.hostname }) port.onMessage.addListener(redirectIfBlacklisted) function redirectIfBlacklisted (response) { const { blacklist } = response - console.log('blacklister contentscript received blacklist response') const host = window.location.hostname if (blacklist && blacklist === host) { window.location.href = 'https://metamask.io/phishing.html' 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.') } } + From eb92d65c4dc764f3a0ad1055fef2ed7cabc04a6f Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Wed, 26 Jul 2017 16:57:58 -0700 Subject: [PATCH 36/53] Add Levenshtein item to changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 823b131be..66c95a0c3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## 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 From 7c71ee1babcaad19dbe7db6c5abfefe2f9654781 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Thu, 27 Jul 2017 15:16:42 -0700 Subject: [PATCH 37/53] Do not blacklist files --- app/manifest.json | 1 - 1 file changed, 1 deletion(-) diff --git a/app/manifest.json b/app/manifest.json index edc4d7162..591a07d0d 100644 --- a/app/manifest.json +++ b/app/manifest.json @@ -56,7 +56,6 @@ { "run_at": "document_start", "matches": [ - "file://*/*", "http://*/*", "https://*/*" ], From 9d345e744d61d94879f3c01a263aa9cf73afa36b Mon Sep 17 00:00:00 2001 From: kumavis Date: Tue, 1 Aug 2017 16:58:40 -0700 Subject: [PATCH 38/53] blacklist - clearer test format --- test/unit/blacklister-test.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/unit/blacklister-test.js b/test/unit/blacklister-test.js index 1badc2c8f..47e9b3c6b 100644 --- a/test/unit/blacklister-test.js +++ b/test/unit/blacklister-test.js @@ -5,19 +5,19 @@ describe('blacklister', function () { describe('#isPhish', function () { it('should not flag whitelisted values', function () { var result = isPhish({ hostname: 'www.metamask.io' }) - assert(!result) + assert.equal(result, false) }) it('should flag explicit values', function () { var result = isPhish({ hostname: 'metamask.com' }) - assert(result) + assert.equal(result, true) }) it('should flag levenshtein values', function () { - var result = isPhish({ hostname: 'metmask.com' }) - assert(result) + var result = isPhish({ hostname: 'metmask.io' }) + assert.equal(result, true) }) it('should not flag not-even-close values', function () { var result = isPhish({ hostname: 'example.com' }) - assert(!result) + assert.equal(result, false) }) }) }) From 9eb13aee0046405bb304a2e31dbf7712e8b1da21 Mon Sep 17 00:00:00 2001 From: kumavis Date: Tue, 1 Aug 2017 17:05:36 -0700 Subject: [PATCH 39/53] blacklist - add tests for metamask subdomains --- test/unit/blacklister-test.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/test/unit/blacklister-test.js b/test/unit/blacklister-test.js index 47e9b3c6b..ce110491c 100644 --- a/test/unit/blacklister-test.js +++ b/test/unit/blacklister-test.js @@ -19,6 +19,18 @@ describe('blacklister', function () { var result = isPhish({ hostname: 'example.com' }) assert.equal(result, false) }) + it('should not flag the ropsten faucet domains', function () { + var result = isPhish({ hostname: 'faucet.metamask.io' }) + assert.equal(result, false) + }) + it('should not flag the mascara domain', function () { + var result = isPhish({ hostname: 'zero.metamask.io' }) + assert.equal(result, false) + }) + it('should not flag the mascara-faucet domain', function () { + var result = isPhish({ hostname: 'zero-faucet.metamask.io' }) + assert.equal(result, false) + }) }) }) From 432f516ab005dd2b4eb4b2e8766ed30216386d98 Mon Sep 17 00:00:00 2001 From: frankiebee Date: Wed, 26 Jul 2017 14:56:52 -0400 Subject: [PATCH 40/53] make addUnapprovedTransaction async function and use promise based ethQuery --- app/scripts/controllers/transactions.js | 296 +++++++++++------------- app/scripts/lib/tx-utils.js | 61 ++--- app/scripts/metamask-controller.js | 29 ++- package.json | 1 + test/unit/tx-controller-test.js | 72 ++++-- test/unit/tx-utils-test.js | 1 + 6 files changed, 219 insertions(+), 241 deletions(-) diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index 5f2d75b47..d3e852ef9 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -1,10 +1,11 @@ const EventEmitter = require('events') const async = require('async') const extend = require('xtend') +const pify = require('pify') 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 getStack = require('../lib/util').getStack const createId = require('../lib/random-id') @@ -33,7 +34,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 @@ -62,13 +63,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 @@ -79,6 +73,50 @@ module.exports = class TransactionController extends EventEmitter { return this.store.getState().transactions } + get unapprovedTxCount () { + return Object.keys(this.getUnapprovedTxList()).length + } + + get pendingTxCount () { + 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 () { + let txList = this.getTxList() + return txList.filter((txMeta) => txMeta.status === 'unapproved') + .reduce((result, tx) => { + result[tx.id] = tx + return result + }, {}) + } + + updateTx (txMeta) { + const txMetaForHistory = clone(txMeta) + txMetaForHistory.stack = getStack() + 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() @@ -92,7 +130,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)) + let index = fullTxList.findIndex((metaTx) => ((metaTx.status === 'confirmed' || metaTx.status === 'rejected') && network === txMeta.metamaskNetworkId)) fullTxList.splice(index, 1) } fullTxList.push(txMeta) @@ -110,86 +148,40 @@ 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) { - const txMetaForHistory = clone(txMeta) - txMetaForHistory.stack = getStack() - var txId = txMeta.id - var txList = this.getFullTxList() - var 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') - } - - 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, - history: [], - } - cb() - }, - // add default tx params - (cb) => this.addTxDefaults(txMeta, cb), - // save txMeta - (cb) => { - this.addTx(txMeta) - cb(null, txMeta) - }, - ], done) + 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 } - addTxDefaults (txMeta, cb) { + 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 - }) + gassPrice = 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) { @@ -221,26 +213,6 @@ module.exports = class TransactionController extends EventEmitter { } } - cancelTransaction (txId, cb = warn) { - this.setTxStatusRejected(txId) - cb() - } - - async updateAndApproveTransaction (txMeta) { - this.updateTx(txMeta) - await this.approveTransaction(txMeta.id) - } - - getChainId () { - const networkState = this.networkStore.getState() - const getChainId = parseInt(networkState) - if (Number.isNaN(getChainId)) { - return 0 - } else { - return getChainId - } - } - async signTransaction (txId) { const txMeta = this.getTx(txId) const txParams = txMeta.txParams @@ -265,6 +237,22 @@ module.exports = class TransactionController extends EventEmitter { }) } + cancelTransaction (txId) { + this.setTxStatusRejected(txId) + return Promise.resolve() + } + + + getChainId () { + const networkState = this.networkStore.getState() + const getChainId = parseInt(networkState) + if (Number.isNaN(getChainId)) { + return 0 + } else { + return getChainId + } + } + // receives a txHash records the tx as signed setTxHash (txId, txHash) { // Add the tx hash to the persisted meta-tx object @@ -275,7 +263,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', @@ -298,7 +286,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) }) @@ -359,7 +347,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) } @@ -367,20 +355,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) { - return this.setTxStatusFailed(txId, { - stack: 'checkForTxInBlock: custom tx-controller error message', - errCode: 'No hash was provided', - message: 'We had an error while submitting this transaction, please try again.', - }) + const noTxHash = new Error('We had an error while submitting this transaction, please try again.') + noTxHash.name = 'NoTxHashError' + this.setTxStatusFailed(noTxHash) } + block.transactions.forEach((tx) => { if (tx.hash === txHash) this.setTxStatusConfirmed(txId) }) @@ -398,6 +385,39 @@ module.exports = class TransactionController extends EventEmitter { 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 // Should find the tx in the tx list and @@ -411,7 +431,7 @@ module.exports = class TransactionController extends EventEmitter { // - `'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) + const txMeta = this.getTx(txId) txMeta.status = status this.emit(`${txMeta.id}:${status}`, txId) if (status === 'submitted' || status === 'rejected') { @@ -436,39 +456,6 @@ module.exports = class TransactionController extends EventEmitter { this.memStore.updateState({ unapprovedTxs, selectedAddressTxList }) } - 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, - }) - })) - } - async _resubmitTx (txMeta) { const address = txMeta.txParams.from const balance = this.ethStore.getState().accounts[address].balance @@ -515,17 +502,14 @@ module.exports = class TransactionController extends EventEmitter { // extra check in case there was an uncaught error during the // signature and submission process if (!txHash) { - this.setTxStatusFailed(txId, { - stack: '_checkPendingTxs: custom tx-controller error message', - errCode: 'No hash was provided', - message: 'We had an error while submitting this transaction, please try again.', - }) - return + const noTxHash = new Error('We had an error while submitting this transaction, please try again.') + noTxHash.name = 'NoTxHashError' + this.setTxStatusFailed(noTxHash) } // 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) @@ -538,12 +522,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/lib/tx-utils.js b/app/scripts/lib/tx-utils.js index 8f6943937..43928feaf 100644 --- a/app/scripts/lib/tx-utils.js +++ b/app/scripts/lib/tx-utils.js @@ -10,24 +10,18 @@ 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) } - 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 await 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 @@ -107,20 +83,17 @@ module.exports = class txProviderUtils { } publishTransaction (rawTx) { - return new Promise((resolve, reject) => { - this.query.sendRawTransaction(rawTx, (err, ress) => { - if (err) reject(err) - else resolve(ress) - }) - }) + return this.query.sendRawTransaction(rawTx) } - validateTxParams (txParams, cb) { - if (('value' in txParams) && txParams.value.indexOf('-') === 0) { - cb(new Error(`Invalid transaction value of ${txParams.value} not a positive number.`)) - } else { - cb() - } + validateTxParams (txParams) { + return new Promise ((resolve, reject) => { + if (('value' in txParams) && txParams.value.indexOf('-') === 0) { + reject(new Error(`Invalid transaction value of ${txParams.value} not a positive number.`)) + } else { + resolve() + } + }) } sufficientBalance (txParams, hexBalance) { diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 11dcde2c1..f7c92e618 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -195,7 +195,7 @@ module.exports = class MetamaskController extends EventEmitter { cb(null, result) }, // tx signing - processTransaction: (txParams, cb) => this.newUnapprovedTransaction(txParams, cb), + processTransaction: nodeify(this.newUnapprovedTransaction, this), // old style msg signing processMessage: this.newUnsignedMessage.bind(this), @@ -308,7 +308,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 @@ -440,22 +440,21 @@ module.exports = class MetamaskController extends EventEmitter { // Identity Management // - newUnapprovedTransaction (txParams, cb) { + async newUnapprovedTransaction (txParams) { 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) => { + const txMeta = await this.txController.addUnapprovedTransaction(txParams) + this.sendUpdate() + this.opts.showUnapprovedTx(txMeta) + // listen for tx completion (success, fail) + return new Promise ((resolve, reject) => { + this.txController.once(`${txMeta.id}:finished`, (completedTx) => { switch (completedTx.status) { case 'submitted': - return cb(null, completedTx.hash) + return reoslve(completedTx.hash) case 'rejected': - return cb(new Error('MetaMask Tx Signature: User denied transaction signature.')) + return reject(new Error('MetaMask Tx Signature: User denied transaction signature.')) default: - return cb(new Error(`MetaMask Tx Signature: Unknown problem: ${JSON.stringify(completedTx.txParams)}`)) + return reject(new Error(`MetaMask Tx Signature: Unknown problem: ${JSON.stringify(completedTx.txParams)}`)) } }) }) @@ -646,6 +645,4 @@ module.exports = class MetamaskController extends EventEmitter { return Promise.resolve(rpcTarget) }) } - - -} +} \ No newline at end of file diff --git a/package.json b/package.json index dcd25cda6..94232d46d 100644 --- a/package.json +++ b/package.json @@ -77,6 +77,7 @@ "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", diff --git a/test/unit/tx-controller-test.js b/test/unit/tx-controller-test.js index 31908569a..6e42e8e68 100644 --- a/test/unit/tx-controller-test.js +++ b/test/unit/tx-controller-test.js @@ -1,7 +1,6 @@ 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') @@ -11,7 +10,7 @@ const currentNetworkId = 42 const otherNetworkId = 36 const privKey = new Buffer('8718b9618a37d1fc78c436511fc6df3c8258d3250635bba617f33003270ec03e', 'hex') -describe('Transaction Controller', function () { +describe.only('Transaction Controller', function () { let txController beforeEach(function () { @@ -20,7 +19,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 +26,59 @@ describe('Transaction Controller', function () { }), }) txController.nonceTracker.getNonceLock = () => Promise.resolve({ nextNonce: 0, releaseLock: noop }) + const queryStubResult = {} + txController.query = new Proxy({}, { + get: (_, key) => { + if (key === 'stubResult') { + return function (method, ...args) { + queryStubResult[method] = args + } + } else { + returnValue = queryStubResult[key] + return () => Promise.resolve(...returnValue) + } + }, + }) + }) + + 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('#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() }) }) - 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 +89,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 +306,15 @@ describe('Transaction Controller', function () { txController.addTx(txMeta) - const estimateStub = sinon.stub(txController.txProviderUtils.query, 'estimateGas') - .callsArgWithAsync(1, null, wrongValue) - - const priceStub = sinon.stub(txController.txProviderUtils.query, 'gasPrice') - .callsArgWithAsync(0, null, wrongValue) + txController.query.stubResult('estimateGas', wrongValue) + txController.query.stubResult('gasPrice', 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 +323,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() @@ -352,7 +378,7 @@ describe('Transaction Controller', function () { }) .catch((err) => { assert.ifError(err, 'should not throw an error') - done() + done(err) }) }) }) diff --git a/test/unit/tx-utils-test.js b/test/unit/tx-utils-test.js index a43bcfb35..43807922a 100644 --- a/test/unit/tx-utils-test.js +++ b/test/unit/tx-utils-test.js @@ -1,4 +1,5 @@ const assert = require('assert') +const sinon = require('sinon') const ethUtil = require('ethereumjs-util') const BN = ethUtil.BN From 21e76484d628d7861b09f013116121101e67334c Mon Sep 17 00:00:00 2001 From: frankiebee Date: Wed, 2 Aug 2017 11:34:45 -0400 Subject: [PATCH 41/53] add test for addTxDefaults --- test/unit/tx-controller-test.js | 35 ++++++++++++++++++++++++++++----- 1 file changed, 30 insertions(+), 5 deletions(-) diff --git a/test/unit/tx-controller-test.js b/test/unit/tx-controller-test.js index 6e42e8e68..fc70da9a2 100644 --- a/test/unit/tx-controller-test.js +++ b/test/unit/tx-controller-test.js @@ -5,12 +5,13 @@ 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 const privKey = new Buffer('8718b9618a37d1fc78c436511fc6df3c8258d3250635bba617f33003270ec03e', 'hex') -describe.only('Transaction Controller', function () { +describe('Transaction Controller', function () { let txController beforeEach(function () { @@ -26,19 +27,19 @@ describe.only('Transaction Controller', function () { }), }) txController.nonceTracker.getNonceLock = () => Promise.resolve({ nextNonce: 0, releaseLock: noop }) - const queryStubResult = {} txController.query = new Proxy({}, { - get: (_, key) => { + get: (queryStubResult, key) => { if (key === 'stubResult') { return function (method, ...args) { queryStubResult[method] = args } } else { - returnValue = queryStubResult[key] - return () => Promise.resolve(...returnValue) + const returnValues = queryStubResult[key] + return () => Promise.resolve(...returnValues) } }, }) + txController.txProviderUtils = new TxProvideUtils(txController.query) }) describe('#addUnapprovedTransaction', function () { @@ -60,6 +61,30 @@ describe.only('Transaction Controller', function () { }) }) + 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('does not throw for positive values', function (done) { var sample = { From 3a4726018e43157909f8c04e03f33cee2584795a Mon Sep 17 00:00:00 2001 From: frankiebee Date: Wed, 2 Aug 2017 11:35:35 -0400 Subject: [PATCH 42/53] fix addTxDefaults --- app/scripts/controllers/transactions.js | 2 +- app/scripts/lib/tx-utils.js | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index d3e852ef9..43dfb9360 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -172,7 +172,7 @@ module.exports = class TransactionController extends EventEmitter { // ensure value txParams.value = txParams.value || '0x0' if (!txParams.gasPrice) { - gassPrice = await this.query.gasPrice() + const gasPrice = await this.query.gasPrice() txParams.gasPrice = gasPrice } // set gasLimit diff --git a/app/scripts/lib/tx-utils.js b/app/scripts/lib/tx-utils.js index 43928feaf..5b9fe0167 100644 --- a/app/scripts/lib/tx-utils.js +++ b/app/scripts/lib/tx-utils.js @@ -19,6 +19,7 @@ module.exports = class txProvideUtils { const block = await this.query.getBlockByNumber('latest', true) const estimatedGasHex = await this.estimateTxGas(txMeta, block.gasLimit) this.setTxGas(txMeta, block.gasLimit, estimatedGasHex) + return txMeta } async estimateTxGas (txMeta, blockGasLimitHex) { @@ -32,7 +33,7 @@ module.exports = class txProvideUtils { txParams.gas = bnToHex(saferGasLimitBN) } // run tx, see if it will OOG - return await this.query.estimateGas(txParams) + return this.query.estimateGas(txParams) } setTxGas (txMeta, blockGasLimitHex, estimatedGasHex) { From ece9200c72cd0b53ee237263933b2b9a24ae5802 Mon Sep 17 00:00:00 2001 From: frankiebee Date: Wed, 2 Aug 2017 11:36:07 -0400 Subject: [PATCH 43/53] fix spelling mistake --- app/scripts/metamask-controller.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index f7c92e618..af4ea32e3 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -450,7 +450,7 @@ module.exports = class MetamaskController extends EventEmitter { this.txController.once(`${txMeta.id}:finished`, (completedTx) => { switch (completedTx.status) { case 'submitted': - return reoslve(completedTx.hash) + return resolve(completedTx.hash) case 'rejected': return reject(new Error('MetaMask Tx Signature: User denied transaction signature.')) default: From 25bc15ba175091513f6d281e9a16f3643fe4b18d Mon Sep 17 00:00:00 2001 From: frankiebee Date: Wed, 2 Aug 2017 11:47:13 -0400 Subject: [PATCH 44/53] lint fixes --- app/scripts/controllers/transactions.js | 6 ++---- app/scripts/lib/tx-utils.js | 7 +------ app/scripts/metamask-controller.js | 2 +- 3 files changed, 4 insertions(+), 11 deletions(-) diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index 8855dfd5b..0c63a5647 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -1,11 +1,9 @@ const EventEmitter = require('events') -const async = require('async') const extend = require('xtend') -const pify = require('pify') const clone = require('clone') const ObservableStore = require('obs-store') const ethUtil = require('ethereumjs-util') -const EthQuery = require('ethjs-query'); +const EthQuery = require('ethjs-query') const TxProviderUtil = require('../lib/tx-utils') const getStack = require('../lib/util').getStack const createId = require('../lib/random-id') @@ -168,7 +166,7 @@ module.exports = class TransactionController extends EventEmitter { history: [], } // add default tx params - await this.addTxDefaults(txMeta), + await this.addTxDefaults(txMeta) // save txMeta this.addTx(txMeta) return txMeta diff --git a/app/scripts/lib/tx-utils.js b/app/scripts/lib/tx-utils.js index 5b9fe0167..24ea2d763 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 @@ -88,7 +87,7 @@ module.exports = class txProvideUtils { } validateTxParams (txParams) { - return new Promise ((resolve, reject) => { + return new Promise((resolve, reject) => { if (('value' in txParams) && txParams.value.indexOf('-') === 0) { reject(new Error(`Invalid transaction value of ${txParams.value} not a positive number.`)) } else { @@ -111,10 +110,6 @@ module.exports = class txProvideUtils { // util -function isUndef (value) { - return value === undefined -} - function bnToHex (inputBn) { return ethUtil.addHexPrefix(inputBn.toString(16)) } diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index af4ea32e3..46a01c900 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -446,7 +446,7 @@ module.exports = class MetamaskController extends EventEmitter { this.sendUpdate() this.opts.showUnapprovedTx(txMeta) // listen for tx completion (success, fail) - return new Promise ((resolve, reject) => { + return new Promise((resolve, reject) => { this.txController.once(`${txMeta.id}:finished`, (completedTx) => { switch (completedTx.status) { case 'submitted': From aea5735b29c87d0a9aad3bfa86d854ed9b20bdf7 Mon Sep 17 00:00:00 2001 From: kumavis Date: Wed, 2 Aug 2017 14:25:28 -0700 Subject: [PATCH 45/53] obj-multiplex - missing name error + prefer const over var --- app/scripts/lib/obj-multiplex.js | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) 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, From ecaa235b5e3331defab75dad72593951fdf37790 Mon Sep 17 00:00:00 2001 From: kumavis Date: Wed, 2 Aug 2017 14:26:10 -0700 Subject: [PATCH 46/53] phishing detection - move phishing detection into contentscript and metamask controller --- app/manifest.json | 9 ----- app/scripts/background.js | 34 +++---------------- app/scripts/blacklister.js | 14 -------- app/scripts/contentscript.js | 22 ++++++++---- app/scripts/lib/inpage-provider.js | 3 ++ app/scripts/lib/is-phish.js | 6 ++-- app/scripts/metamask-controller.js | 26 ++++++++++++-- gulpfile.js | 1 - package.json | 2 +- ...ter-test.js => phishing-detection-test.js} | 2 +- 10 files changed, 52 insertions(+), 67 deletions(-) delete mode 100644 app/scripts/blacklister.js rename test/unit/{blacklister-test.js => phishing-detection-test.js} (96%) diff --git a/app/manifest.json b/app/manifest.json index 591a07d0d..1eaf6f26a 100644 --- a/app/manifest.json +++ b/app/manifest.json @@ -52,15 +52,6 @@ ], "run_at": "document_start", "all_frames": true - }, - { - "run_at": "document_start", - "matches": [ - "http://*/*", - "https://*/*" - ], - "js": ["scripts/blacklister.js"], - "all_frames": true } ], "permissions": [ diff --git a/app/scripts/background.js b/app/scripts/background.js index bc0fbdc37..a235afff3 100644 --- a/app/scripts/background.js +++ b/app/scripts/background.js @@ -11,7 +11,6 @@ const NotificationManager = require('./lib/notification-manager.js') const MetamaskController = require('./metamask-controller') const extension = require('extensionizer') const firstTimeState = require('./first-time-state') -const isPhish = require('./lib/is-phish') const STORAGE_KEY = 'metamask-config' const METAMASK_DEBUG = 'GULP_METAMASK_DEBUG' @@ -91,16 +90,12 @@ function setupController (initState) { extension.runtime.onConnect.addListener(connectRemote) function connectRemote (remotePort) { - if (remotePort.name === 'blacklister') { - return checkBlacklist(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, () => { @@ -109,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) } } @@ -140,27 +135,6 @@ function setupController (initState) { return Promise.resolve() } -// Listen for new pages and return if blacklisted: -function checkBlacklist (port) { - const handler = handleNewPageLoad.bind(null, port) - port.onMessage.addListener(handler) - setTimeout(() => { - port.onMessage.removeListener(handler) - }, 30000) -} - -function handleNewPageLoad (port, message) { - const { pageLoaded } = message - if (!pageLoaded || !global.metamaskController) return - - const state = global.metamaskController.getState() - const updatedBlacklist = state.blacklist - - if (isPhish({ updatedBlacklist, hostname: pageLoaded })) { - port.postMessage({ 'blacklist': pageLoaded }) - } -} - // // Etc... // diff --git a/app/scripts/blacklister.js b/app/scripts/blacklister.js deleted file mode 100644 index 37751b595..000000000 --- a/app/scripts/blacklister.js +++ /dev/null @@ -1,14 +0,0 @@ -const extension = require('extensionizer') - -var port = extension.runtime.connect({name: 'blacklister'}) -port.postMessage({ 'pageLoaded': window.location.hostname }) -port.onMessage.addListener(redirectIfBlacklisted) - -function redirectIfBlacklisted (response) { - const { blacklist } = response - const host = window.location.hostname - if (blacklist && blacklist === host) { - window.location.href = 'https://metamask.io/phishing.html' - } -} - diff --git a/app/scripts/contentscript.js b/app/scripts/contentscript.js index 291b922e8..6fde0edcd 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/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/is-phish.js b/app/scripts/lib/is-phish.js index 68c09e4ac..21c68b0d6 100644 --- a/app/scripts/lib/is-phish.js +++ b/app/scripts/lib/is-phish.js @@ -9,15 +9,15 @@ const LEVENSHTEIN_CHECKS = ['myetherwallet', 'myetheroll', 'ledgerwallet', 'meta // credit to @sogoiii and @409H for their help! // Return a boolean on whether or not a phish is detected. -function isPhish({ hostname, updatedBlacklist = null }) { +function isPhish({ hostname, blacklist }) { var strCurrentTab = hostname // check if the domain is part of the whitelist. if (whitelistedDomains && whitelistedDomains.includes(strCurrentTab)) { return false } // Allow updating of blacklist: - if (updatedBlacklist) { - blacklistedDomains = blacklistedDomains.concat(updatedBlacklist) + if (blacklist) { + blacklistedDomains = blacklistedDomains.concat(blacklist) } // check if the domain is part of the blacklist. diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 11dcde2c1..28c35a13d 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -23,6 +23,7 @@ const ConfigManager = require('./lib/config-manager') const nodeify = require('./lib/nodeify') const accountImporter = require('./account-import-strategies') const getBuyEthUrl = require('./lib/buy-eth-url') +const checkForPhishing = require('./lib/is-phish') const debounce = require('debounce') const version = require('../manifest.json').version @@ -326,8 +327,15 @@ module.exports = class MetamaskController extends EventEmitter { } setupUntrustedCommunication (connectionStream, originDomain) { + // Check if new connection is blacklisted + if (this.isHostBlacklisted(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 +343,26 @@ 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) } + // Check if a domain is on our blacklist + isHostBlacklisted (hostname) { + if (!hostname) return false + const { blacklist } = this.getState().blacklist + return checkForPhishing({ blacklist, hostname }) + } + + sendPhishingWarning (connectionStream, hostname) { + const mx = setupMultiplex(connectionStream) + const phishingStream = mx.createStream('phishing') + // phishingStream.write(true) + phishingStream.write({ hostname }) + } + setupControllerConnection (outStream) { const api = this.getApi() const dnode = Dnode(api) 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 10afc8228..40bc78c6f 100644 --- a/package.json +++ b/package.json @@ -126,7 +126,7 @@ "sw-stream": "^2.0.0", "textarea-caret": "^3.0.1", "three.js": "^0.73.2", - "through2": "^2.0.1", + "through2": "^2.0.3", "valid-url": "^1.0.9", "vreme": "^3.0.2", "web3": "0.19.1", diff --git a/test/unit/blacklister-test.js b/test/unit/phishing-detection-test.js similarity index 96% rename from test/unit/blacklister-test.js rename to test/unit/phishing-detection-test.js index ce110491c..affcb16c2 100644 --- a/test/unit/blacklister-test.js +++ b/test/unit/phishing-detection-test.js @@ -1,7 +1,7 @@ const assert = require('assert') const isPhish = require('../../app/scripts/lib/is-phish') -describe('blacklister', function () { +describe('phishing detection test', function () { describe('#isPhish', function () { it('should not flag whitelisted values', function () { var result = isPhish({ hostname: 'www.metamask.io' }) From 8c6f01b91094564df59d6d95b6f43b811e711824 Mon Sep 17 00:00:00 2001 From: kumavis Date: Wed, 2 Aug 2017 15:54:59 -0700 Subject: [PATCH 47/53] blacklist controller - breakout from metamask and infura controllers --- app/scripts/controllers/blacklist.js | 50 ++++++++++++++++++++++++++ app/scripts/controllers/infura.js | 16 +-------- app/scripts/lib/is-phish.js | 29 ++++----------- app/scripts/metamask-controller.js | 19 +++++----- test/unit/blacklist-controller-test.js | 41 +++++++++++++++++++++ test/unit/phishing-detection-test.js | 36 ------------------- 6 files changed, 108 insertions(+), 83 deletions(-) create mode 100644 app/scripts/controllers/blacklist.js create mode 100644 test/unit/blacklist-controller-test.js delete mode 100644 test/unit/phishing-detection-test.js diff --git a/app/scripts/controllers/blacklist.js b/app/scripts/controllers/blacklist.js new file mode 100644 index 000000000..11e26d5b2 --- /dev/null +++ b/app/scripts/controllers/blacklist.js @@ -0,0 +1,50 @@ +const ObservableStore = require('obs-store') +const extend = require('xtend') +const communityBlacklistedDomains = require('etheraddresslookup/blacklists/domains.json') +const communityWhitelistedDomains = require('etheraddresslookup/whitelists/domains.json') +const checkForPhishing = require('../lib/is-phish') + +// compute phishing lists +const PHISHING_BLACKLIST = communityBlacklistedDomains.concat(['metamask.com']) +const PHISHING_WHITELIST = communityWhitelistedDomains.concat(['metamask.io', 'www.metamask.io']) +const PHISHING_FUZZYLIST = ['myetherwallet', 'myetheroll', 'ledgerwallet', 'metamask'] +// every ten minutes +const POLLING_INTERVAL = 10 * 60 * 1000 + +class BlacklistController { + + constructor (opts = {}) { + const initState = extend({ + phishing: PHISHING_BLACKLIST, + }, opts.initState) + this.store = new ObservableStore(initState) + // polling references + this._phishingUpdateIntervalRef = null + } + + // + // PUBLIC METHODS + // + + checkForPhishing (hostname) { + if (!hostname) return false + const { blacklist } = this.store.getState() + return checkForPhishing({ hostname, blacklist, whitelist: PHISHING_WHITELIST, fuzzylist: PHISHING_FUZZYLIST }) + } + + async updatePhishingList () { + const response = await fetch('https://api.infura.io/v1/blacklist') + const phishing = await response.json() + this.store.updateState({ phishing }) + return phishing + } + + scheduleUpdates () { + if (this._phishingUpdateIntervalRef) return + this._phishingUpdateIntervalRef = setInterval(() => { + this.updatePhishingList() + }, POLLING_INTERVAL) + } +} + +module.exports = BlacklistController diff --git a/app/scripts/controllers/infura.js b/app/scripts/controllers/infura.js index 97b2ab7e3..10adb1004 100644 --- a/app/scripts/controllers/infura.js +++ b/app/scripts/controllers/infura.js @@ -1,16 +1,14 @@ const ObservableStore = require('obs-store') const extend = require('xtend') -const recentBlacklist = require('etheraddresslookup/blacklists/domains.json') // every ten minutes -const POLLING_INTERVAL = 300000 +const POLLING_INTERVAL = 10 * 60 * 1000 class InfuraController { constructor (opts = {}) { const initState = extend({ infuraNetworkStatus: {}, - blacklist: recentBlacklist, }, opts.initState) this.store = new ObservableStore(initState) } @@ -32,24 +30,12 @@ class InfuraController { }) } - updateLocalBlacklist () { - return fetch('https://api.infura.io/v1/blacklist') - .then(response => response.json()) - .then((parsedResponse) => { - this.store.updateState({ - blacklist: parsedResponse, - }) - return parsedResponse - }) - } - scheduleInfuraNetworkCheck () { if (this.conversionInterval) { clearInterval(this.conversionInterval) } this.conversionInterval = setInterval(() => { this.checkInfuraNetworkStatus() - this.updateLocalBlacklist() }, POLLING_INTERVAL) } } diff --git a/app/scripts/lib/is-phish.js b/app/scripts/lib/is-phish.js index 21c68b0d6..ce51c353d 100644 --- a/app/scripts/lib/is-phish.js +++ b/app/scripts/lib/is-phish.js @@ -1,38 +1,23 @@ const levenshtein = require('fast-levenshtein') -const blacklistedMetaMaskDomains = ['metamask.com'] -let blacklistedDomains = require('etheraddresslookup/blacklists/domains.json').concat(blacklistedMetaMaskDomains) -const whitelistedMetaMaskDomains = ['metamask.io', 'www.metamask.io'] -const whitelistedDomains = require('etheraddresslookup/whitelists/domains.json').concat(whitelistedMetaMaskDomains) const LEVENSHTEIN_TOLERANCE = 4 -const LEVENSHTEIN_CHECKS = ['myetherwallet', 'myetheroll', 'ledgerwallet', 'metamask'] - // credit to @sogoiii and @409H for their help! // Return a boolean on whether or not a phish is detected. -function isPhish({ hostname, blacklist }) { - var strCurrentTab = hostname +function isPhish({ hostname, blacklist, whitelist, fuzzylist }) { // check if the domain is part of the whitelist. - if (whitelistedDomains && whitelistedDomains.includes(strCurrentTab)) { return false } - - // Allow updating of blacklist: - if (blacklist) { - blacklistedDomains = blacklistedDomains.concat(blacklist) - } + if (whitelist && whitelist.includes(hostname)) return false // check if the domain is part of the blacklist. - const isBlacklisted = blacklistedDomains && blacklistedDomains.includes(strCurrentTab) + if (blacklist && blacklist.includes(hostname)) return true // check for similar values. - let levenshteinMatched = false - var levenshteinForm = strCurrentTab.replace(/\./g, '') - LEVENSHTEIN_CHECKS.forEach((element) => { - if (levenshtein.get(element, levenshteinForm) <= LEVENSHTEIN_TOLERANCE) { - levenshteinMatched = true - } + const levenshteinForm = hostname.replace(/\./g, '') + const levenshteinMatched = fuzzylist.some((element) => { + return levenshtein.get(element, levenshteinForm) <= LEVENSHTEIN_TOLERANCE }) - return isBlacklisted || levenshteinMatched + return levenshteinMatched } module.exports = isPhish diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 28c35a13d..6d6cb85ab 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') @@ -23,7 +24,6 @@ const ConfigManager = require('./lib/config-manager') const nodeify = require('./lib/nodeify') const accountImporter = require('./account-import-strategies') const getBuyEthUrl = require('./lib/buy-eth-url') -const checkForPhishing = require('./lib/is-phish') const debounce = require('debounce') const version = require('../manifest.json').version @@ -70,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() @@ -152,6 +156,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 }) }) @@ -328,7 +335,7 @@ module.exports = class MetamaskController extends EventEmitter { setupUntrustedCommunication (connectionStream, originDomain) { // Check if new connection is blacklisted - if (this.isHostBlacklisted(originDomain)) { + if (this.blacklistController.checkForPhishing(originDomain)) { console.log('MetaMask - sending phishing warning for', originDomain) this.sendPhishingWarning(connectionStream, originDomain) return @@ -349,17 +356,9 @@ module.exports = class MetamaskController extends EventEmitter { this.setupProviderConnection(mx.createStream('provider'), originDomain) } - // Check if a domain is on our blacklist - isHostBlacklisted (hostname) { - if (!hostname) return false - const { blacklist } = this.getState().blacklist - return checkForPhishing({ blacklist, hostname }) - } - sendPhishingWarning (connectionStream, hostname) { const mx = setupMultiplex(connectionStream) const phishingStream = mx.createStream('phishing') - // phishingStream.write(true) phishingStream.write({ hostname }) } 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/phishing-detection-test.js b/test/unit/phishing-detection-test.js deleted file mode 100644 index affcb16c2..000000000 --- a/test/unit/phishing-detection-test.js +++ /dev/null @@ -1,36 +0,0 @@ -const assert = require('assert') -const isPhish = require('../../app/scripts/lib/is-phish') - -describe('phishing detection test', function () { - describe('#isPhish', function () { - it('should not flag whitelisted values', function () { - var result = isPhish({ hostname: 'www.metamask.io' }) - assert.equal(result, false) - }) - it('should flag explicit values', function () { - var result = isPhish({ hostname: 'metamask.com' }) - assert.equal(result, true) - }) - it('should flag levenshtein values', function () { - var result = isPhish({ hostname: 'metmask.io' }) - assert.equal(result, true) - }) - it('should not flag not-even-close values', function () { - var result = isPhish({ hostname: 'example.com' }) - assert.equal(result, false) - }) - it('should not flag the ropsten faucet domains', function () { - var result = isPhish({ hostname: 'faucet.metamask.io' }) - assert.equal(result, false) - }) - it('should not flag the mascara domain', function () { - var result = isPhish({ hostname: 'zero.metamask.io' }) - assert.equal(result, false) - }) - it('should not flag the mascara-faucet domain', function () { - var result = isPhish({ hostname: 'zero-faucet.metamask.io' }) - assert.equal(result, false) - }) - }) -}) - From b80c7e417bfa3adf338170472ba4c4c6733e8402 Mon Sep 17 00:00:00 2001 From: frankiebee Date: Wed, 2 Aug 2017 18:58:05 -0400 Subject: [PATCH 48/53] move newUnapprovedTransaction to transactions.js --- app/scripts/background.js | 2 +- app/scripts/controllers/transactions.js | 63 ++++++++++++++-------- app/scripts/lib/tx-utils.js | 12 ++--- app/scripts/metamask-controller.js | 23 +-------- test/unit/tx-controller-test.js | 69 +++++++++++++++++++++++-- 5 files changed, 115 insertions(+), 54 deletions(-) diff --git a/app/scripts/background.js b/app/scripts/background.js index bc0fbdc37..f995c390e 100644 --- a/app/scripts/background.js +++ b/app/scripts/background.js @@ -126,7 +126,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/controllers/transactions.js b/app/scripts/controllers/transactions.js index 0c63a5647..720323e41 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -70,11 +70,11 @@ module.exports = class TransactionController extends EventEmitter { return this.store.getState().transactions } - get unapprovedTxCount () { + getUnapprovedTxCount () { return Object.keys(this.getUnapprovedTxList()).length } - get pendingTxCount () { + getPendingTxCount () { return this.getTxsByMetaData('status', 'signed').length } @@ -92,7 +92,7 @@ module.exports = class TransactionController extends EventEmitter { return txMeta } getUnapprovedTxList () { - let txList = this.getTxList() + const txList = this.getTxList() return txList.filter((txMeta) => txMeta.status === 'unapproved') .reduce((result, tx) => { result[tx.id] = tx @@ -135,7 +135,7 @@ module.exports = class TransactionController extends EventEmitter { // or rejected tx's. // not tx's that are pending or unapproved if (txCount > txHistoryLimit - 1) { - let 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) @@ -153,6 +153,25 @@ module.exports = class TransactionController extends EventEmitter { this.emit(`${txMeta.id}:unapproved`, txMeta) } + 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)}`)) + } + }) + }) + } + async addUnapprovedTransaction (txParams) { // validate await this.txProviderUtils.validateTxParams(txParams) @@ -229,10 +248,9 @@ module.exports = class TransactionController extends EventEmitter { // 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()) - }) + await this.signEthTx(ethTx, fromAddress) + this.setTxStatusSigned(txMeta.id) + const rawTx = ethUtil.bufferToHex(ethTx.serialize()) return rawTx } @@ -240,15 +258,13 @@ module.exports = class TransactionController extends EventEmitter { 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) - }) + const txHash = await this.txProviderUtils.publishTransaction(rawTx) + this.setTxHash(txId, txHash) + this.setTxStatusSubmitted(txId) } - cancelTransaction (txId) { + async cancelTransaction (txId) { this.setTxStatusRejected(txId) - return Promise.resolve() } @@ -371,9 +387,9 @@ module.exports = class TransactionController extends EventEmitter { const txId = txMeta.id if (!txHash) { - const noTxHash = new Error('We had an error while submitting this transaction, please try again.') - noTxHash.name = 'NoTxHashError' - this.setTxStatusFailed(noTxHash) + const noTxHashErr = new Error('We had an error while submitting this transaction, please try again.') + noTxHashErr.name = 'NoTxHashError' + this.setTxStatusFailed(txId, noTxHashErr) } @@ -427,7 +443,12 @@ module.exports = class TransactionController extends EventEmitter { })) } - // PRIVATE METHODS + +/* _____________________________________ +| | +| PRIVATE METHODS | +|______________________________________*/ + // Should find the tx in the tx list and // update it. @@ -511,9 +532,9 @@ module.exports = class TransactionController extends EventEmitter { // extra check in case there was an uncaught error during the // signature and submission process if (!txHash) { - const noTxHash = new Error('We had an error while submitting this transaction, please try again.') - noTxHash.name = 'NoTxHashError' - this.setTxStatusFailed(noTxHash) + 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 diff --git a/app/scripts/lib/tx-utils.js b/app/scripts/lib/tx-utils.js index 24ea2d763..0e5b6a999 100644 --- a/app/scripts/lib/tx-utils.js +++ b/app/scripts/lib/tx-utils.js @@ -86,14 +86,10 @@ module.exports = class txProvideUtils { return this.query.sendRawTransaction(rawTx) } - validateTxParams (txParams) { - return new Promise((resolve, reject) => { - if (('value' in txParams) && txParams.value.indexOf('-') === 0) { - reject(new Error(`Invalid transaction value of ${txParams.value} not a positive number.`)) - } else { - resolve() - } - }) + async validateTxParams (txParams) { + if (('value' in txParams) && txParams.value.indexOf('-') === 0) { + throw new Error(`Invalid transaction value of ${txParams.value} not a positive number.`) + } } sufficientBalance (txParams, hexBalance) { diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 46a01c900..794ca1a9b 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -108,6 +108,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({ @@ -195,7 +196,7 @@ module.exports = class MetamaskController extends EventEmitter { cb(null, result) }, // tx signing - processTransaction: nodeify(this.newUnapprovedTransaction, this), + processTransaction: nodeify(async (txParams) => await this.txController.newUnapprovedTransaction(txParams), this), // old style msg signing processMessage: this.newUnsignedMessage.bind(this), @@ -440,26 +441,6 @@ module.exports = class MetamaskController extends EventEmitter { // Identity Management // - async newUnapprovedTransaction (txParams) { - log.debug(`MetaMaskController newUnapprovedTransaction ${JSON.stringify(txParams)}`) - const txMeta = await this.txController.addUnapprovedTransaction(txParams) - this.sendUpdate() - this.opts.showUnapprovedTx(txMeta) - // listen for tx completion (success, fail) - return new Promise((resolve, reject) => { - this.txController.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)}`)) - } - }) - }) - } - newUnsignedMessage (msgParams, cb) { const msgId = this.messageManager.addUnapprovedMessage(msgParams) this.sendUpdate() diff --git a/test/unit/tx-controller-test.js b/test/unit/tx-controller-test.js index fc70da9a2..c7743a7c6 100644 --- a/test/unit/tx-controller-test.js +++ b/test/unit/tx-controller-test.js @@ -42,6 +42,70 @@ describe('Transaction Controller', function () { 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) @@ -92,7 +156,7 @@ describe('Transaction Controller', function () { } txController.txProviderUtils.validateTxParams(sample).then(() => { done() - }) + }).catch(done) }) it('returns error for negative values', function (done) { @@ -407,5 +471,4 @@ describe('Transaction Controller', function () { }) }) }) -}) - +}) \ No newline at end of file From 0808eb22562ac9a64b00cb5aa54856ac8c1ea18c Mon Sep 17 00:00:00 2001 From: frankiebee Date: Wed, 2 Aug 2017 19:07:18 -0400 Subject: [PATCH 49/53] fix test --- test/unit/tx-controller-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/tx-controller-test.js b/test/unit/tx-controller-test.js index c7743a7c6..f290088a1 100644 --- a/test/unit/tx-controller-test.js +++ b/test/unit/tx-controller-test.js @@ -108,7 +108,7 @@ describe('Transaction Controller', function () { 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) + const addTxDefaultsStub = sinon.stub(txController, 'addTxDefaults').callsFake(() => Promise.resolve()) txController.addUnapprovedTransaction({}) .then((txMeta) => { assert(('id' in txMeta), 'should have a id') From 340dbe75fca95373b861372d89ecdb246ad0e681 Mon Sep 17 00:00:00 2001 From: frankiebee Date: Wed, 2 Aug 2017 19:09:37 -0400 Subject: [PATCH 50/53] use async with #publishTransaction --- app/scripts/lib/tx-utils.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/scripts/lib/tx-utils.js b/app/scripts/lib/tx-utils.js index 0e5b6a999..3687a9652 100644 --- a/app/scripts/lib/tx-utils.js +++ b/app/scripts/lib/tx-utils.js @@ -82,8 +82,8 @@ module.exports = class txProvideUtils { return ethTx } - publishTransaction (rawTx) { - return this.query.sendRawTransaction(rawTx) + async publishTransaction (rawTx) { + return await this.query.sendRawTransaction(rawTx) } async validateTxParams (txParams) { From 5ac4c2de6fa3a83709eeefc71d5afc0857a28445 Mon Sep 17 00:00:00 2001 From: frankiebee Date: Wed, 2 Aug 2017 19:11:51 -0400 Subject: [PATCH 51/53] remove unused sinon --- test/unit/tx-utils-test.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/unit/tx-utils-test.js b/test/unit/tx-utils-test.js index 43807922a..a43bcfb35 100644 --- a/test/unit/tx-utils-test.js +++ b/test/unit/tx-utils-test.js @@ -1,5 +1,4 @@ const assert = require('assert') -const sinon = require('sinon') const ethUtil = require('ethereumjs-util') const BN = ethUtil.BN From dce593fd7bbe1848b8744e3911f24d9ac2fa1bf7 Mon Sep 17 00:00:00 2001 From: frankiebee Date: Thu, 3 Aug 2017 15:39:55 -0400 Subject: [PATCH 52/53] remove stack from txs --- app/scripts/controllers/transactions.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index 720323e41..308d43cb0 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -5,7 +5,6 @@ const ObservableStore = require('obs-store') const ethUtil = require('ethereumjs-util') const EthQuery = require('ethjs-query') const TxProviderUtil = require('../lib/tx-utils') -const getStack = require('../lib/util').getStack const createId = require('../lib/random-id') const NonceTracker = require('../lib/nonce-tracker') @@ -105,8 +104,6 @@ module.exports = class TransactionController extends EventEmitter { const txMetaForHistory = clone(txMeta) // dont include previous history in this snapshot delete txMetaForHistory.history - // add stack to help understand why tx was updated - txMetaForHistory.stack = getStack() // add snapshot to tx history if (!txMeta.history) txMeta.history = [] txMeta.history.push(txMetaForHistory) From d4877cb4e2580db565686e7c736cd3716dc6e02d Mon Sep 17 00:00:00 2001 From: kumavis Date: Thu, 3 Aug 2017 14:25:02 -0700 Subject: [PATCH 53/53] blacklist - use module eth-phishing-detect --- app/scripts/controllers/blacklist.js | 28 ++++++++++++++++++---------- app/scripts/lib/is-phish.js | 23 ----------------------- package.json | 4 ++-- 3 files changed, 20 insertions(+), 35 deletions(-) delete mode 100644 app/scripts/lib/is-phish.js diff --git a/app/scripts/controllers/blacklist.js b/app/scripts/controllers/blacklist.js index 11e26d5b2..7e01fa386 100644 --- a/app/scripts/controllers/blacklist.js +++ b/app/scripts/controllers/blacklist.js @@ -1,13 +1,9 @@ const ObservableStore = require('obs-store') const extend = require('xtend') -const communityBlacklistedDomains = require('etheraddresslookup/blacklists/domains.json') -const communityWhitelistedDomains = require('etheraddresslookup/whitelists/domains.json') -const checkForPhishing = require('../lib/is-phish') +const PhishingDetector = require('eth-phishing-detect/src/detector') // compute phishing lists -const PHISHING_BLACKLIST = communityBlacklistedDomains.concat(['metamask.com']) -const PHISHING_WHITELIST = communityWhitelistedDomains.concat(['metamask.io', 'www.metamask.io']) -const PHISHING_FUZZYLIST = ['myetherwallet', 'myetheroll', 'ledgerwallet', 'metamask'] +const PHISHING_DETECTION_CONFIG = require('eth-phishing-detect/src/config.json') // every ten minutes const POLLING_INTERVAL = 10 * 60 * 1000 @@ -15,9 +11,12 @@ class BlacklistController { constructor (opts = {}) { const initState = extend({ - phishing: PHISHING_BLACKLIST, + 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 } @@ -28,14 +27,15 @@ class BlacklistController { checkForPhishing (hostname) { if (!hostname) return false - const { blacklist } = this.store.getState() - return checkForPhishing({ hostname, blacklist, whitelist: PHISHING_WHITELIST, fuzzylist: PHISHING_FUZZYLIST }) + const { result } = this._phishingDetector.check(hostname) + return result } async updatePhishingList () { - const response = await fetch('https://api.infura.io/v1/blacklist') + const response = await fetch('https://api.infura.io/v2/blacklist') const phishing = await response.json() this.store.updateState({ phishing }) + this._setupPhishingDetector(phishing) return phishing } @@ -45,6 +45,14 @@ class BlacklistController { this.updatePhishingList() }, POLLING_INTERVAL) } + + // + // PRIVATE METHODS + // + + _setupPhishingDetector (config) { + this._phishingDetector = new PhishingDetector(config) + } } module.exports = BlacklistController diff --git a/app/scripts/lib/is-phish.js b/app/scripts/lib/is-phish.js deleted file mode 100644 index ce51c353d..000000000 --- a/app/scripts/lib/is-phish.js +++ /dev/null @@ -1,23 +0,0 @@ -const levenshtein = require('fast-levenshtein') -const LEVENSHTEIN_TOLERANCE = 4 - -// credit to @sogoiii and @409H for their help! -// Return a boolean on whether or not a phish is detected. -function isPhish({ hostname, blacklist, whitelist, fuzzylist }) { - - // check if the domain is part of the whitelist. - if (whitelist && whitelist.includes(hostname)) return false - - // check if the domain is part of the blacklist. - if (blacklist && blacklist.includes(hostname)) return true - - // check for similar values. - const levenshteinForm = hostname.replace(/\./g, '') - const levenshteinMatched = fuzzylist.some((element) => { - return levenshtein.get(element, levenshteinForm) <= LEVENSHTEIN_TOLERANCE - }) - - return levenshteinMatched -} - -module.exports = isPhish diff --git a/package.json b/package.json index a086af29d..a85da614c 100644 --- a/package.json +++ b/package.json @@ -7,7 +7,7 @@ "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\"", @@ -68,11 +68,11 @@ "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.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",