diff options
Merge branch 'master' into NewUI
27 files changed, 630 insertions, 214 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index e7934dc77..bf18bb361 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,8 +2,38 @@ ## 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. +- 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 + +- Now detects and blocks known phishing sites. + +## 3.8.6 2017-7-11 + +- Make transaction resubmission more resilient. +- No longer validate nonce client-side in retry loop. +- Fix bug where insufficient balance error was sometimes shown on successful transactions. + +## 3.8.5 2017-7-7 + +- Fix transaction resubmit logic to fail slightly less eagerly. + +## 3.8.4 2017-7-7 + +- Improve transaction resubmit logic to fail more eagerly when a user would expect it to. + +## 3.8.3 2017-7-6 + - Re-enable default token list. - Add origin header to dapp-bound requests to allow providers to throttle sites. +- Fix bug that could sometimes resubmit a transaction that had been stalled due to low balance after balance was restored. ## 3.8.2 2017-7-3 @@ -1,10 +1,20 @@ -# MetaMask Plugin [](https://circleci.com/gh/MetaMask/metamask-plugin) +# MetaMask Plugin [](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: -- If you've never built a Dapp before, we've got a gentle introduction on [Developing Dapps with Truffle and MetaMask](https://blog.metamask.io/developing-for-metamask-with-truffle/). +### New Dapp Developers + +- We recommend this [Learning Solidity](https://karl.tech/learning-solidity-part-1-deploy-a-contract/) tutorial series by Karl Floersch. +- We wrote a (slightly outdated now) gentle introduction on [Developing Dapps with Truffle and MetaMask](https://medium.com/metamask/developing-ethereum-dapps-with-truffle-and-metamask-aa8ad7e363ba). + +### Current Dapp Developers + - If you have a Dapp, and you want to ensure compatibility, [here is our guide on building MetaMask-compatible Dapps](https://github.com/MetaMask/faq/blob/master/DEVELOPERS.md) ## Building locally diff --git a/app/manifest.json b/app/manifest.json index 12ff6c2ea..eadd99590 100644 --- a/app/manifest.json +++ b/app/manifest.json @@ -1,7 +1,7 @@ { "name": "MetaMask", "short_name": "Metamask", - "version": "3.8.2", + "version": "3.9.1", "manifest_version": 2, "author": "https://metamask.io", "description": "Ethereum Browser Extension", @@ -52,6 +52,11 @@ ], "run_at": "document_start", "all_frames": true + }, + { + "run_at": "document_start", + "matches": ["http://*/*", "https://*/*"], + "js": ["scripts/blacklister.js"] } ], "permissions": [ diff --git a/app/scripts/blacklister.js b/app/scripts/blacklister.js new file mode 100644 index 000000000..a45265a75 --- /dev/null +++ b/app/scripts/blacklister.js @@ -0,0 +1,13 @@ +const blacklistedDomains = require('etheraddresslookup/blacklists/domains.json')
+
+function detectBlacklistedDomain() {
+ var strCurrentTab = window.location.hostname
+ if (blacklistedDomains && blacklistedDomains.includes(strCurrentTab)) {
+ window.location.href = 'https://metamask.io/phishing.html'
+ }
+}
+
+window.addEventListener('load', function() {
+ detectBlacklistedDomain()
+})
+
diff --git a/app/scripts/controllers/infura.js b/app/scripts/controllers/infura.js index 98375b446..b34b0bc03 100644 --- a/app/scripts/controllers/infura.js +++ b/app/scripts/controllers/infura.js @@ -26,6 +26,7 @@ class InfuraController { this.store.updateState({ infuraNetworkStatus: parsedResponse, }) + return parsedResponse }) } diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index 52251d66e..5f3d84ebe 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -1,12 +1,12 @@ const EventEmitter = require('events') const async = require('async') const extend = require('xtend') -const Semaphore = require('semaphore') const ObservableStore = require('obs-store') const ethUtil = require('ethereumjs-util') +const pify = require('pify') const TxProviderUtil = require('../lib/tx-utils') const createId = require('../lib/random-id') -const denodeify = require('denodeify') +const NonceTracker = require('../lib/nonce-tracker') module.exports = class TransactionController extends EventEmitter { constructor (opts) { @@ -20,13 +20,26 @@ module.exports = class TransactionController extends EventEmitter { this.txHistoryLimit = opts.txHistoryLimit this.provider = opts.provider this.blockTracker = opts.blockTracker + this.nonceTracker = new NonceTracker({ + provider: this.provider, + blockTracker: this.provider._blockTracker, + getPendingTransactions: (address) => { + return this.getFilteredTxList({ + from: address, + status: 'submitted', + err: undefined, + }) + }, + }) this.query = opts.ethQuery this.txProviderUtils = new TxProviderUtil(this.query) this.blockTracker.on('rawBlock', this.checkForTxInBlock.bind(this)) - this.blockTracker.on('latest', this.resubmitPendingTxs.bind(this)) + // this is a little messy but until ethstore has been either + // removed or redone this is to guard against the race condition + // where ethStore hasent been populated by the results yet + this.blockTracker.once('latest', () => this.blockTracker.on('latest', this.resubmitPendingTxs.bind(this))) this.blockTracker.on('sync', this.queryPendingTxs.bind(this)) this.signEthTx = opts.signTransaction - this.nonceLock = Semaphore(1) this.ethStore = opts.ethStore // memstore is computed from a few different stores this._updateMemstore() @@ -170,29 +183,32 @@ module.exports = class TransactionController extends EventEmitter { }, {}) } - approveTransaction (txId, cb = warn) { - const self = this - // approve - self.setTxStatusApproved(txId) - // only allow one tx at a time for atomic nonce usage - self.nonceLock.take(() => { - // begin signature process - async.waterfall([ - (cb) => self.fillInTxParams(txId, cb), - (cb) => self.signTransaction(txId, cb), - (rawTx, cb) => self.publishTransaction(txId, rawTx, cb), - ], (err) => { - self.nonceLock.leave() - if (err) { - this.setTxStatusFailed(txId, { - errCode: err.errCode || err, - message: err.message || 'Transaction failed during approval', - }) - return cb(err) - } - cb() + async approveTransaction (txId) { + let nonceLock + try { + // approve + this.setTxStatusApproved(txId) + // get next nonce + const txMeta = this.getTx(txId) + const fromAddress = txMeta.txParams.from + nonceLock = await this.nonceTracker.getNonceLock(fromAddress) + txMeta.txParams.nonce = nonceLock.nextNonce + this.updateTx(txMeta) + // sign transaction + const rawTx = await this.signTransaction(txId) + await this.publishTransaction(txId, rawTx) + // must set transaction to submitted/failed before releasing lock + nonceLock.releaseLock() + } catch (err) { + this.setTxStatusFailed(txId, { + errCode: err.errCode || err, + message: err.message || 'Transaction failed during approval', }) - }) + // must set transaction to submitted/failed before releasing lock + if (nonceLock) nonceLock.releaseLock() + // continue with error chain + throw err + } } cancelTransaction (txId, cb = warn) { @@ -200,13 +216,9 @@ module.exports = class TransactionController extends EventEmitter { cb() } - fillInTxParams (txId, cb) { - const txMeta = this.getTx(txId) - this.txProviderUtils.fillInTxParams(txMeta.txParams, (err) => { - if (err) return cb(err) - this.updateTx(txMeta) - cb() - }) + async updateAndApproveTransaction (txMeta) { + this.updateTx(txMeta) + await this.approveTransaction(txMeta.id) } getChainId () { @@ -219,31 +231,27 @@ module.exports = class TransactionController extends EventEmitter { } } - signTransaction (txId, cb) { + async signTransaction (txId) { const txMeta = this.getTx(txId) const txParams = txMeta.txParams const fromAddress = txParams.from // add network/chain id txParams.chainId = this.getChainId() const ethTx = this.txProviderUtils.buildEthTxFromParams(txParams) - this.signEthTx(ethTx, fromAddress).then(() => { + const rawTx = await this.signEthTx(ethTx, fromAddress).then(() => { this.setTxStatusSigned(txMeta.id) - cb(null, ethUtil.bufferToHex(ethTx.serialize())) - }).catch((err) => { - cb(err) + return ethUtil.bufferToHex(ethTx.serialize()) }) + return rawTx } - publishTransaction (txId, rawTx, cb = warn) { + async publishTransaction (txId, rawTx) { const txMeta = this.getTx(txId) txMeta.rawTx = rawTx this.updateTx(txMeta) - - this.txProviderUtils.publishTransaction(rawTx, (err, txHash) => { - if (err) return cb(err) + await this.txProviderUtils.publishTransaction(rawTx).then((txHash) => { this.setTxHash(txId, txHash) this.setTxStatusSubmitted(txId) - cb() }) } @@ -261,10 +269,19 @@ module.exports = class TransactionController extends EventEmitter { to: '0x0..', from: '0x0..', status: 'signed', + err: undefined, } and returns a list of tx with all options matching + ****************HINT**************** + | `err: undefined` is like looking | + | for a tx with no err | + | so you can also search txs that | + | dont have something as well by | + | setting the value as undefined | + ************************************ + this is for things like filtering a the tx list for only tx's from 1 account or for filltering for all txs from one account @@ -413,65 +430,103 @@ module.exports = class TransactionController extends EventEmitter { const pending = this.getTxsByMetaData('status', 'submitted') // only try resubmitting if their are transactions to resubmit if (!pending.length) return - const resubmit = denodeify(this._resubmitTx.bind(this)) - Promise.all(pending.map(txMeta => resubmit(txMeta))) - .catch((reason) => { - log.info('Problem resubmitting tx', reason) - }) + 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, { + errCode: err.errCode || err, + message: err.message, + }) + })) } - _resubmitTx (txMeta, cb) { + async _resubmitTx (txMeta, cb) { const address = txMeta.txParams.from const balance = this.ethStore.getState().accounts[address].balance - const nonce = Number.parseInt(this.ethStore.getState().accounts[address].nonce) - const txNonce = Number.parseInt(txMeta.txParams.nonce) - const gtBalance = Number.parseInt(txMeta.txParams.value) > Number.parseInt(balance) if (!('retryCount' in txMeta)) txMeta.retryCount = 0 - // if the value of the transaction is greater then the balance - // or the nonce of the transaction is lower then the accounts nonce - // dont resubmit the tx - if (gtBalance || txNonce < nonce) return cb() + // if the value of the transaction is greater then the balance, fail. + if (!this.txProviderUtils.sufficientBalance(txMeta.txParams, balance)) { + const message = 'Insufficient balance.' + this.setTxStatusFailed(txMeta.id, { message }) + cb() + return log.error(message) + } + // Only auto-submit already-signed txs: if (!('rawTx' in txMeta)) return cb() // Increment a try counter. txMeta.retryCount++ const rawTx = txMeta.rawTx - this.txProviderUtils.publishTransaction(rawTx, cb) + return await this.txProviderUtils.publishTransaction(rawTx, cb) } // checks the network for signed txs and // if confirmed sets the tx status as 'confirmed' - _checkPendingTxs () { - var signedTxList = this.getFilteredTxList({status: 'submitted'}) - if (!signedTxList.length) return - signedTxList.forEach((txMeta) => { - var txHash = txMeta.hash - var txId = txMeta.id - if (!txHash) { - const errReason = { - errCode: 'No hash was provided', - message: 'We had an error while submitting this transaction, please try again.', - } - return this.setTxStatusFailed(txId, errReason) + async _checkPendingTxs () { + const signedTxList = this.getFilteredTxList({status: 'submitted'}) + // in order to keep the nonceTracker accurate we block it while updating pending transactions + const nonceGlobalLock = await this.nonceTracker.getGlobalLock() + try { + await Promise.all(signedTxList.map((txMeta) => this._checkPendingTx(txMeta))) + } catch (err) { + console.error('TransactionController - Error updating pending transactions') + console.error(err) + } + nonceGlobalLock.releaseLock() + } + + async _checkPendingTx (txMeta) { + const txHash = txMeta.hash + const txId = txMeta.id + // extra check in case there was an uncaught error during the + // signature and submission process + if (!txHash) { + const errReason = { + errCode: 'No hash was provided', + message: 'We had an error while submitting this transaction, please try again.', } - this.query.getTransactionByHash(txHash, (err, txParams) => { - if (err || !txParams) { - if (!txParams) return - txMeta.err = { - isWarning: true, - errorCode: err, - message: 'There was a problem loading this transaction.', - } - this.updateTx(txMeta) - return log.error(err) - } - if (txParams.blockNumber) { - this.setTxStatusConfirmed(txId) + this.setTxStatusFailed(txId, errReason) + return + } + // get latest transaction status + let txParams + try { + txParams = await pify((cb) => this.query.getTransactionByHash(txHash, cb))() + if (!txParams) return + if (txParams.blockNumber) { + this.setTxStatusConfirmed(txId) + } + } catch (err) { + if (err || !txParams) { + txMeta.err = { + isWarning: true, + errorCode: err, + message: 'There was a problem loading this transaction.', } - }) - }) + this.updateTx(txMeta) + log.error(err) + } + } } } diff --git a/app/scripts/lib/auto-faucet.js b/app/scripts/lib/auto-faucet.js deleted file mode 100644 index 38d54ba5e..000000000 --- a/app/scripts/lib/auto-faucet.js +++ /dev/null @@ -1,20 +0,0 @@ -const uri = 'https://faucet.metamask.io/' -const METAMASK_DEBUG = 'GULP_METAMASK_DEBUG' -const env = process.env.METAMASK_ENV - -module.exports = function (address) { - // Don't faucet in development or test - if (METAMASK_DEBUG === true || env === 'test') return - global.log.info('auto-fauceting:', address) - const data = address - const headers = new Headers() - headers.append('Content-type', 'application/rawdata') - fetch(uri, { - method: 'POST', - headers, - body: data, - }) - .catch((err) => { - console.error(err) - }) -} diff --git a/app/scripts/lib/nodeify.js b/app/scripts/lib/nodeify.js index 51d89a8fb..299bfe624 100644 --- a/app/scripts/lib/nodeify.js +++ b/app/scripts/lib/nodeify.js @@ -1,24 +1,9 @@ -module.exports = function (promiseFn) { - return function () { - var args = [] - for (var i = 0; i < arguments.length - 1; i++) { - args.push(arguments[i]) - } - var cb = arguments[arguments.length - 1] +const promiseToCallback = require('promise-to-callback') - const nodeified = promiseFn.apply(this, args) - - if (!nodeified) { - const methodName = String(promiseFn).split('(')[0] - throw new Error(`The ${methodName} did not return a Promise, but was nodeified.`) - } - nodeified.then(function (result) { - cb(null, result) - }) - .catch(function (reason) { - cb(reason) - }) - - return nodeified +module.exports = function(fn, context) { + return function(){ + const args = [].slice.call(arguments) + const callback = args.pop() + promiseToCallback(fn.apply(context, args))(callback) } } diff --git a/app/scripts/lib/nonce-tracker.js b/app/scripts/lib/nonce-tracker.js new file mode 100644 index 000000000..b76dac4e8 --- /dev/null +++ b/app/scripts/lib/nonce-tracker.js @@ -0,0 +1,84 @@ +const EthQuery = require('eth-query') +const assert = require('assert') +const Mutex = require('await-semaphore').Mutex + +class NonceTracker { + + constructor ({ blockTracker, provider, getPendingTransactions }) { + this.blockTracker = blockTracker + this.ethQuery = new EthQuery(provider) + this.getPendingTransactions = getPendingTransactions + this.lockMap = {} + } + + async getGlobalLock () { + const globalMutex = this._lookupMutex('global') + // await global mutex free + const releaseLock = await globalMutex.acquire() + return { releaseLock } + } + + // releaseLock must be called + // releaseLock must be called after adding signed tx to pending transactions (or discarding) + async getNonceLock (address) { + // await global mutex free + await this._globalMutexFree() + // await lock free, then take lock + const releaseLock = await this._takeMutex(address) + // calculate next nonce + // we need to make sure our base count + // and pending count are from the same block + const currentBlock = await this._getCurrentBlock() + const pendingTransactions = this.getPendingTransactions(address) + const pendingCount = pendingTransactions.length + assert(Number.isInteger(pendingCount), 'nonce-tracker - pendingCount is an integer') + const baseCountHex = await this._getTxCount(address, currentBlock) + const baseCount = parseInt(baseCountHex, 16) + assert(Number.isInteger(baseCount), 'nonce-tracker - baseCount is an integer') + const nextNonce = baseCount + pendingCount + assert(Number.isInteger(nextNonce), 'nonce-tracker - nextNonce is an integer') + // return next nonce and release cb + return { nextNonce, releaseLock } + } + + async _getCurrentBlock () { + const currentBlock = this.blockTracker.getCurrentBlock() + if (currentBlock) return currentBlock + return await Promise((reject, resolve) => { + this.blockTracker.once('latest', resolve) + }) + } + + async _getTxCount (address, currentBlock) { + const blockNumber = currentBlock.number + return new Promise((resolve, reject) => { + this.ethQuery.getTransactionCount(address, blockNumber, (err, result) => { + err ? reject(err) : resolve(result) + }) + }) + } + + async _globalMutexFree () { + const globalMutex = this._lookupMutex('global') + const release = await globalMutex.acquire() + release() + } + + async _takeMutex (lockId) { + const mutex = this._lookupMutex(lockId) + const releaseLock = await mutex.acquire() + return releaseLock + } + + _lookupMutex (lockId) { + let mutex = this.lockMap[lockId] + if (!mutex) { + mutex = new Mutex() + this.lockMap[lockId] = mutex + } + return mutex + } + +} + +module.exports = NonceTracker diff --git a/app/scripts/lib/tx-utils.js b/app/scripts/lib/tx-utils.js index 149d93102..8f6943937 100644 --- a/app/scripts/lib/tx-utils.js +++ b/app/scripts/lib/tx-utils.js @@ -106,8 +106,13 @@ module.exports = class txProviderUtils { return ethTx } - publishTransaction (rawTx, cb) { - this.query.sendRawTransaction(rawTx, cb) + publishTransaction (rawTx) { + return new Promise((resolve, reject) => { + this.query.sendRawTransaction(rawTx, (err, ress) => { + if (err) reject(err) + else resolve(ress) + }) + }) } validateTxParams (txParams, cb) { @@ -118,6 +123,15 @@ module.exports = class txProviderUtils { } } + sufficientBalance (txParams, hexBalance) { + const balance = hexToBn(hexBalance) + const value = hexToBn(txParams.value) + const gasLimit = hexToBn(txParams.gas) + const gasPrice = hexToBn(txParams.gasPrice) + + const maxCost = value.add(gasLimit.mul(gasPrice)) + return balance.gte(maxCost) + } } diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 73093dfad..11dcde2c1 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -20,7 +20,6 @@ const MessageManager = require('./lib/message-manager') const PersonalMessageManager = require('./lib/personal-message-manager') const TransactionController = require('./controllers/transactions') const ConfigManager = require('./lib/config-manager') -const autoFaucet = require('./lib/auto-faucet') const nodeify = require('./lib/nodeify') const accountImporter = require('./account-import-strategies') const getBuyEthUrl = require('./lib/buy-eth-url') @@ -90,9 +89,6 @@ module.exports = class MetamaskController extends EventEmitter { this.keyringController.on('newAccount', (address) => { this.preferencesController.setSelectedAddress(address) }) - this.keyringController.on('newVault', (address) => { - autoFaucet(address) - }) // address book controller this.addressBookController = new AddressBookController({ @@ -294,34 +290,33 @@ module.exports = class MetamaskController extends EventEmitter { submitPassword: this.submitPassword.bind(this), // PreferencesController - setSelectedAddress: nodeify(preferencesController.setSelectedAddress).bind(preferencesController), - addToken: nodeify(preferencesController.addToken).bind(preferencesController), - setCurrentAccountTab: nodeify(preferencesController.setCurrentAccountTab).bind(preferencesController), - setDefaultRpc: nodeify(this.setDefaultRpc).bind(this), - setCustomRpc: nodeify(this.setCustomRpc).bind(this), + setSelectedAddress: nodeify(preferencesController.setSelectedAddress, preferencesController), + addToken: nodeify(preferencesController.addToken, preferencesController), + setCurrentAccountTab: nodeify(preferencesController.setCurrentAccountTab, preferencesController), + setDefaultRpc: nodeify(this.setDefaultRpc, this), + setCustomRpc: nodeify(this.setCustomRpc, this), // AddressController - setAddressBook: nodeify(addressBookController.setAddressBook).bind(addressBookController), + setAddressBook: nodeify(addressBookController.setAddressBook, addressBookController), // KeyringController - setLocked: nodeify(keyringController.setLocked).bind(keyringController), - createNewVaultAndKeychain: nodeify(keyringController.createNewVaultAndKeychain).bind(keyringController), - createNewVaultAndRestore: nodeify(keyringController.createNewVaultAndRestore).bind(keyringController), - addNewKeyring: nodeify(keyringController.addNewKeyring).bind(keyringController), - saveAccountLabel: nodeify(keyringController.saveAccountLabel).bind(keyringController), - exportAccount: nodeify(keyringController.exportAccount).bind(keyringController), + setLocked: nodeify(keyringController.setLocked, keyringController), + createNewVaultAndKeychain: nodeify(keyringController.createNewVaultAndKeychain, keyringController), + createNewVaultAndRestore: nodeify(keyringController.createNewVaultAndRestore, keyringController), + addNewKeyring: nodeify(keyringController.addNewKeyring, keyringController), + saveAccountLabel: nodeify(keyringController.saveAccountLabel, keyringController), + exportAccount: nodeify(keyringController.exportAccount, keyringController), // txController - approveTransaction: txController.approveTransaction.bind(txController), cancelTransaction: txController.cancelTransaction.bind(txController), - updateAndApproveTransaction: this.updateAndApproveTx.bind(this), + updateAndApproveTransaction: nodeify(txController.updateAndApproveTransaction, txController), // messageManager - signMessage: nodeify(this.signMessage).bind(this), + signMessage: nodeify(this.signMessage, this), cancelMessage: this.cancelMessage.bind(this), // personalMessageManager - signPersonalMessage: nodeify(this.signPersonalMessage).bind(this), + signPersonalMessage: nodeify(this.signPersonalMessage, this), cancelPersonalMessage: this.cancelPersonalMessage.bind(this), // notices @@ -367,7 +362,7 @@ module.exports = class MetamaskController extends EventEmitter { function onResponse (err, request, response) { if (err) return console.error(err) if (response.error) { - console.error('Error in RPC response:\n', response.error) + console.error('Error in RPC response:\n', response) } if (request.isMetamaskInternal) return log.info(`RPC (${originDomain}):`, request, '->', response) @@ -502,13 +497,6 @@ module.exports = class MetamaskController extends EventEmitter { }) } - updateAndApproveTx (txMeta, cb) { - log.debug(`MetaMaskController - updateAndApproveTx: ${JSON.stringify(txMeta)}`) - const txController = this.txController - txController.updateTx(txMeta) - txController.approveTransaction(txMeta.id, cb) - } - signMessage (msgParams, cb) { log.info('MetaMaskController - signMessage') const msgId = msgParams.metamaskId diff --git a/app/scripts/migrations/016.js b/app/scripts/migrations/016.js new file mode 100644 index 000000000..4fc534f1c --- /dev/null +++ b/app/scripts/migrations/016.js @@ -0,0 +1,41 @@ +const version = 16 + +/* + +This migration sets transactions with the 'Gave up submitting tx.' err message +to a 'failed' stated + +*/ + +const clone = require('clone') + +module.exports = { + version, + + migrate: function (originalVersionedData) { + const versionedData = clone(originalVersionedData) + versionedData.meta.version = version + try { + const state = versionedData.data + const newState = transformState(state) + versionedData.data = newState + } catch (err) { + console.warn(`MetaMask Migration #${version}` + err.stack) + } + return Promise.resolve(versionedData) + }, +} + +function transformState (state) { + const newState = state + const transactions = newState.TransactionController.transactions + newState.TransactionController.transactions = transactions.map((txMeta) => { + if (!txMeta.err) return txMeta + if (txMeta.err === 'transaction with the same hash was already imported.') { + txMeta.status = 'submitted' + delete txMeta.err + } + return txMeta + }) + return newState +} diff --git a/app/scripts/migrations/017.js b/app/scripts/migrations/017.js new file mode 100644 index 000000000..24959cd3a --- /dev/null +++ b/app/scripts/migrations/017.js @@ -0,0 +1,40 @@ +const version = 17 + +/* + +This migration sets transactions who were retried and marked as failed to submitted + +*/ + +const clone = require('clone') + +module.exports = { + version, + + migrate: function (originalVersionedData) { + const versionedData = clone(originalVersionedData) + versionedData.meta.version = version + try { + const state = versionedData.data + const newState = transformState(state) + versionedData.data = newState + } catch (err) { + console.warn(`MetaMask Migration #${version}` + err.stack) + } + return Promise.resolve(versionedData) + }, +} + +function transformState (state) { + const newState = state + const transactions = newState.TransactionController.transactions + newState.TransactionController.transactions = transactions.map((txMeta) => { + if (!txMeta.status === 'failed') return txMeta + if (txMeta.retryCount > 0 && txMeta.retryCount < 2) { + txMeta.status = 'submitted' + delete txMeta.err + } + return txMeta + }) + return newState +} diff --git a/app/scripts/migrations/index.js b/app/scripts/migrations/index.js index 651ee6a9c..f4c87499f 100644 --- a/app/scripts/migrations/index.js +++ b/app/scripts/migrations/index.js @@ -26,4 +26,6 @@ module.exports = [ require('./013'), require('./014'), require('./015'), + require('./016'), + require('./017'), ] diff --git a/circle.yml b/circle.yml index 1f018ac24..66eed17d7 100644 --- a/circle.yml +++ b/circle.yml @@ -1,6 +1,6 @@ machine: node: - version: 8.0.0 + version: 8.1.4 dependencies: pre: - "npm i -g testem" diff --git a/gulpfile.js b/gulpfile.js index 628314b37..f0a28e273 100644 --- a/gulpfile.js +++ b/gulpfile.js @@ -172,6 +172,7 @@ gulp.task('default', ['lint'], function () { const jsFiles = [ 'inpage', 'contentscript', + 'blacklister', 'background', 'popup', 'responsive' diff --git a/package.json b/package.json index 0129583f7..a95f2c75f 100644 --- a/package.json +++ b/package.json @@ -7,10 +7,12 @@ "start": "npm run dev", "dev": "gulp dev --debug", "disc": "gulp disc --debug", - "dist": "npm install && gulp dist", + "clear": "rm -rf node_modules/eth-contract-metadata && rm -rf node_modules/etheraddresslookup", + "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\"", "test-responsive": "METAMASK_ENV=test mocha --require test/helper.js --recursive \"test/unit/responsive/**/*.js\"", + "single-test": "METAMASK_ENV=test mocha --require test/helper.js", "test-integration": "npm run buildMock && npm run buildCiUnits && testem ci -P 2", "lint": "gulp lint", "buildCiUnits": "node test/integration/index.js", @@ -46,6 +48,7 @@ }, "dependencies": { "async": "^1.5.2", + "await-semaphore": "^0.1.1", "babel-runtime": "^6.23.0", "bip39": "^2.2.0", "bluebird": "^3.5.0", @@ -56,19 +59,19 @@ "copy-to-clipboard": "^2.0.0", "debounce": "^1.0.0", "deep-extend": "^0.4.1", - "denodeify": "^1.2.1", "detect-node": "^2.0.3", "disc": "^1.3.2", "dnode": "^1.2.2", "end-of-stream": "^1.1.0", "ensnare": "^1.0.0", "eth-bin-to-ops": "^1.0.1", - "eth-contract-metadata": "^1.1.3", + "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", "ethereumjs-tx": "^1.3.0", "ethereumjs-util": "ethereumjs/ethereumjs-util#ac5d0908536b447083ea422b435da27f26615de9", "ethereumjs-wallet": "^0.6.0", @@ -125,7 +128,7 @@ "valid-url": "^1.0.9", "vreme": "^3.0.2", "web3": "0.19.1", - "web3-provider-engine": "^13.1.1", + "web3-provider-engine": "^13.2.8", "web3-stream-provider": "^3.0.1", "xtend": "^4.0.1" }, @@ -175,7 +178,7 @@ "react-addons-test-utils": "^15.5.1", "react-test-renderer": "^15.5.4", "react-testutils-additions": "^15.2.0", - "sinon": "^1.17.3", + "sinon": "^2.3.8", "tape": "^4.5.1", "testem": "^1.10.3", "uglifyify": "^3.0.1", diff --git a/test/unit/infura-controller-test.js b/test/unit/infura-controller-test.js index 7a2a114f9..605305efa 100644 --- a/test/unit/infura-controller-test.js +++ b/test/unit/infura-controller-test.js @@ -1,33 +1,61 @@ -// polyfill fetch -global.fetch = function () {return Promise.resolve({ - json: () => { return Promise.resolve({"mainnet": "ok", "ropsten": "degraded", "kovan": "down", "rinkeby": "ok"}) }, - }) -} const assert = require('assert') +const sinon = require('sinon') const InfuraController = require('../../app/scripts/controllers/infura') describe('infura-controller', function () { - var infuraController + let infuraController, sandbox, networkStatus + const response = {'mainnet': 'degraded', 'ropsten': 'ok', 'kovan': 'ok', 'rinkeby': 'down'} - beforeEach(function () { + before(async function () { infuraController = new InfuraController() + sandbox = sinon.sandbox.create() + sinon.stub(infuraController, 'checkInfuraNetworkStatus').resolves(response) + networkStatus = await infuraController.checkInfuraNetworkStatus() + }) + + after(function () { + sandbox.restore() }) - describe('network status queries', function () { - describe('#checkInfuraNetworkStatus', function () { - it('should return an object reflecting the network statuses', function (done) { - this.timeout(15000) - infuraController.checkInfuraNetworkStatus() - .then(() => { - const networkStatus = infuraController.store.getState().infuraNetworkStatus - assert.equal(Object.keys(networkStatus).length, 4) - assert.equal(networkStatus.mainnet, 'ok') - assert.equal(networkStatus.ropsten, 'degraded') - assert.equal(networkStatus.kovan, 'down') - }) - .then(() => done()) - .catch(done) + describe('Network status queries', function () { + + describe('Mainnet', function () { + it('should have Mainnet', function () { + assert.equal(Object.keys(networkStatus)[0], 'mainnet') + }) + + it('should have a value for Mainnet status', function () { + assert.equal(networkStatus.mainnet, 'degraded') + }) + }) + + describe('Ropsten', function () { + it('should have Ropsten', function () { + assert.equal(Object.keys(networkStatus)[1], 'ropsten') + }) + + it('should have a value for Ropsten status', function () { + assert.equal(networkStatus.ropsten, 'ok') + }) + }) + + describe('Kovan', function () { + it('should have Kovan', function () { + assert.equal(Object.keys(networkStatus)[2], 'kovan') + }) + + it('should have a value for Kovan status', function () { + assert.equal(networkStatus.kovan, 'ok') + }) + }) + + describe('Rinkeby', function () { + it('should have Rinkeby', function () { + assert.equal(Object.keys(networkStatus)[3], 'rinkeby') + }) + it('should have a value for Rinkeby status', function () { + assert.equal(networkStatus.rinkeby, 'down') }) }) }) diff --git a/test/unit/nodeify-test.js b/test/unit/nodeify-test.js index 5aed758fa..06241334d 100644 --- a/test/unit/nodeify-test.js +++ b/test/unit/nodeify-test.js @@ -11,7 +11,7 @@ describe('nodeify', function () { } it('should retain original context', function (done) { - var nodified = nodeify(obj.promiseFunc).bind(obj) + var nodified = nodeify(obj.promiseFunc, obj) nodified('baz', function (err, res) { assert.equal(res, 'barbaz') done() diff --git a/test/unit/nonce-tracker-test.js b/test/unit/nonce-tracker-test.js new file mode 100644 index 000000000..16cd6d008 --- /dev/null +++ b/test/unit/nonce-tracker-test.js @@ -0,0 +1,40 @@ +const assert = require('assert') +const NonceTracker = require('../../app/scripts/lib/nonce-tracker') + +describe('Nonce Tracker', function () { + let nonceTracker, provider, getPendingTransactions, pendingTxs + + + beforeEach(function () { + pendingTxs = [{ + 'status': 'submitted', + 'txParams': { + 'from': '0x7d3517b0d011698406d6e0aed8453f0be2697926', + 'gas': '0x30d40', + 'value': '0x0', + 'nonce': '0x0', + }, + }] + + + getPendingTransactions = () => pendingTxs + provider = { sendAsync: (_, cb) => { cb(undefined, {result: '0x0'}) } } + nonceTracker = new NonceTracker({ + blockTracker: { + getCurrentBlock: () => '0x11b568', + }, + provider, + getPendingTransactions, + }) + }) + + describe('#getNonceLock', function () { + it('should work', async function (done) { + this.timeout(15000) + const nonceLock = await nonceTracker.getNonceLock('0x7d3517b0d011698406d6e0aed8453f0be2697926') + assert.equal(nonceLock.nextNonce, '1', 'nonce should be 1') + await nonceLock.releaseLock() + done() + }) + }) +}) diff --git a/test/unit/tx-controller-test.js b/test/unit/tx-controller-test.js index 0d35cd62c..7b86cfe14 100644 --- a/test/unit/tx-controller-test.js +++ b/test/unit/tx-controller-test.js @@ -1,5 +1,4 @@ const assert = require('assert') -const EventEmitter = require('events') const ethUtil = require('ethereumjs-util') const EthTx = require('ethereumjs-tx') const EthQuery = require('eth-query') @@ -19,14 +18,16 @@ describe('Transaction Controller', function () { txController = new TransactionController({ networkStore: new ObservableStore(currentNetworkId), txHistoryLimit: 10, - provider: { _blockTracker: new EventEmitter()}, - blockTracker: new EventEmitter(), - ethQuery: new EthQuery(new EventEmitter()), + 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) resolve() }), }) + txController.nonceTracker.getNonceLock = () => Promise.resolve({ nextNonce: 0, releaseLock: noop }) }) describe('#validateTxParams', function () { @@ -270,56 +271,86 @@ describe('Transaction Controller', function () { it('does not overwrite set values', function (done) { + this.timeout(15000) const wrongValue = '0x05' txController.addTx(txMeta) const estimateStub = sinon.stub(txController.txProviderUtils.query, 'estimateGas') - .callsArgWith(1, null, wrongValue) + .callsArgWithAsync(1, null, wrongValue) const priceStub = sinon.stub(txController.txProviderUtils.query, 'gasPrice') - .callsArgWith(0, null, wrongValue) + .callsArgWithAsync(0, null, wrongValue) - const nonceStub = sinon.stub(txController.txProviderUtils.query, 'getTransactionCount') - .callsArgWith(2, null, wrongValue) - const signStub = sinon.stub(txController, 'signTransaction') - .callsArgWith(1, null, noop) + const signStub = sinon.stub(txController, 'signTransaction', () => Promise.resolve()) - const pubStub = sinon.stub(txController.txProviderUtils, 'publishTransaction') - .callsArgWith(1, null, originalValue) - - txController.approveTransaction(txMeta.id, (err) => { - assert.ifError(err, 'should not error') + const pubStub = sinon.stub(txController.txProviderUtils, 'publishTransaction', () => Promise.resolve(originalValue)) + txController.approveTransaction(txMeta.id).then(() => { const result = txController.getTx(txMeta.id) const params = result.txParams assert.equal(params.gas, originalValue, 'gas unmodified') assert.equal(params.gasPrice, originalValue, 'gas price unmodified') - assert.equal(params.nonce, originalValue, 'nonce unmodified') - assert.equal(result.hash, originalValue, 'hash was set') + assert.equal(result.hash, originalValue, `hash was set \n got: ${result.hash} \n expected: ${originalValue}`) estimateStub.restore() priceStub.restore() signStub.restore() - nonceStub.restore() pubStub.restore() - done() - }) + }).catch(done) }) }) describe('#sign replay-protected tx', function () { it('prepares a tx with the chainId set', function (done) { txController.addTx({ id: '1', status: 'unapproved', metamaskNetworkId: currentNetworkId, txParams: {} }, noop) - txController.signTransaction('1', (err, rawTx) => { - if (err) return done('it should not fail') + txController.signTransaction('1').then((rawTx) => { const ethTx = new EthTx(ethUtil.toBuffer(rawTx)) assert.equal(ethTx.getChainId(), currentNetworkId) done() + }).catch(done) + }) + }) + + describe('#_resubmitTx with a too-low balance', function () { + it('should fail the transaction', function (done) { + const from = '0xda0da0' + const txMeta = { + id: 1, + status: 'submitted', + metamaskNetworkId: currentNetworkId, + txParams: { + from, + nonce: '0x1', + value: '0xfffff', + }, + } + + const lowBalance = '0x0' + const fakeStoreState = { accounts: {} } + fakeStoreState.accounts[from] = { + balance: lowBalance, + nonce: '0x0', + } + + // Stubbing out current account state: + const getStateStub = sinon.stub(txController.ethStore, 'getState') + .returns(fakeStoreState) + + // Adding the fake tx: + txController.addTx(clone(txMeta)) + + txController._resubmitTx(txMeta, function (err) { + assert.ifError(err, 'should not throw an error') + const updatedMeta = txController.getTx(txMeta.id) + assert.notEqual(updatedMeta.status, txMeta.status, 'status changed.') + assert.equal(updatedMeta.status, 'failed', 'tx set to failed.') + done() }) }) }) }) + diff --git a/test/unit/tx-helper-test.js b/test/unit/tx-helper-test.js new file mode 100644 index 000000000..cc6543c30 --- /dev/null +++ b/test/unit/tx-helper-test.js @@ -0,0 +1,17 @@ +const assert = require('assert') +const txHelper = require('../../ui/lib/tx-helper') + +describe('txHelper', function () { + it('always shows the oldest tx first', function () { + const metamaskNetworkId = 1 + const txs = { + a: { metamaskNetworkId, time: 3 }, + b: { metamaskNetworkId, time: 1 }, + c: { metamaskNetworkId, time: 2 }, + } + + const sorted = txHelper(txs, null, null, metamaskNetworkId) + assert.equal(sorted[0].time, 1, 'oldest tx first') + assert.equal(sorted[2].time, 3, 'newest tx last') + }) +}) diff --git a/test/unit/tx-utils-test.js b/test/unit/tx-utils-test.js index 7ace1f587..a43bcfb35 100644 --- a/test/unit/tx-utils-test.js +++ b/test/unit/tx-utils-test.js @@ -16,6 +16,44 @@ describe('txUtils', function () { })) }) + describe('#sufficientBalance', function () { + it('returns true if max tx cost is equal to balance.', function () { + const tx = { + 'value': '0x1', + 'gas': '0x2', + 'gasPrice': '0x3', + } + const balance = '0x8' + + const result = txUtils.sufficientBalance(tx, balance) + assert.ok(result, 'sufficient balance found.') + }) + + it('returns true if max tx cost is less than balance.', function () { + const tx = { + 'value': '0x1', + 'gas': '0x2', + 'gasPrice': '0x3', + } + const balance = '0x9' + + const result = txUtils.sufficientBalance(tx, balance) + assert.ok(result, 'sufficient balance found.') + }) + + it('returns false if max tx cost is more than balance.', function () { + const tx = { + 'value': '0x1', + 'gas': '0x2', + 'gasPrice': '0x3', + } + const balance = '0x6' + + const result = txUtils.sufficientBalance(tx, balance) + assert.ok(!result, 'insufficient balance found.') + }) + }) + describe('chain Id', function () { it('prepares a transaction with the provided chainId', function () { const txParams = { diff --git a/ui/app/add-token.js b/ui/app/add-token.js index b303b5c0d..15ef7a852 100644 --- a/ui/app/add-token.js +++ b/ui/app/add-token.js @@ -86,7 +86,7 @@ AddTokenScreen.prototype.render = function () { h('div', [ h('span', { style: { fontWeight: 'bold', paddingRight: '10px'}, - }, 'Token Sybmol'), + }, 'Token Symbol'), ]), h('div', { style: {display: 'flex'} }, [ diff --git a/ui/app/components/pending-tx.js b/ui/app/components/pending-tx.js index d7d602f31..5324ccd64 100644 --- a/ui/app/components/pending-tx.js +++ b/ui/app/components/pending-tx.js @@ -15,7 +15,7 @@ const addressSummary = util.addressSummary const nameForAddress = require('../../lib/contract-namer') const BNInput = require('./bn-as-decimal-input') -const MIN_GAS_PRICE_GWEI_BN = new BN(2) +const MIN_GAS_PRICE_GWEI_BN = new BN(1) const GWEI_FACTOR = new BN(1e9) const MIN_GAS_PRICE_BN = MIN_GAS_PRICE_GWEI_BN.mul(GWEI_FACTOR) const MIN_GAS_LIMIT_BN = new BN(21000) diff --git a/ui/app/info.js b/ui/app/info.js index e8470de97..cb2e41f5b 100644 --- a/ui/app/info.js +++ b/ui/app/info.js @@ -97,11 +97,17 @@ InfoScreen.prototype.render = function () { paddingLeft: '30px', }}, [ + h('div.fa.fa-support', [ + h('a.info', { + href: 'http://metamask.consensyssupport.happyfox.com', + target: '_blank', + }, 'Visit our Support Center'), + ]), h('div.fa.fa-github', [ h('a.info', { - href: 'https://github.com/MetaMask/faq', + href: 'https://github.com/MetaMask/metamask-extension/issues/new', target: '_blank', - }, 'Need Help? Read our FAQ!'), + }, 'Found a bug? Report it!'), ]), h('div', [ h('a', { diff --git a/ui/lib/tx-helper.js b/ui/lib/tx-helper.js index ec19daf64..afc62e7b6 100644 --- a/ui/lib/tx-helper.js +++ b/ui/lib/tx-helper.js @@ -12,6 +12,10 @@ module.exports = function (unapprovedTxs, unapprovedMsgs, personalMsgs, network) const personalValues = valuesFor(personalMsgs) log.debug(`tx helper found ${personalValues.length} unsigned personal messages`) allValues = allValues.concat(personalValues) + allValues = allValues.sort((a, b) => { + return a.time > b.time + }) - return allValues.sort(txMeta => txMeta.time) + return allValues } + |