From d571f5ee701acd87495ad8ed69a74e6c5ca424f3 Mon Sep 17 00:00:00 2001 From: Thomas Huang Date: Sun, 23 Jul 2017 21:32:49 -0700 Subject: [PATCH 01/29] 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 02/29] 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 03/29] 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 04/29] 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 05/29] 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 06/29] 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 07/29] 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 08/29] 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 09/29] 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 10/29] 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 11/29] 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 12/29] 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 13/29] 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 14/29] 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 15/29] 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 16/29] 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 17/29] 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 18/29] 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 19/29] 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 20/29] 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 21/29] 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 22/29] 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 23/29] 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 24/29] 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 b50c10f373c30642e083fb87724fc5db2ffff1e9 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Wed, 26 Jul 2017 14:15:24 -0700 Subject: [PATCH 25/29] 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 26/29] 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 27/29] 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 28/29] 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 29/29] 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' } })