From c53c5d5c9e2a384c6b0a17b844919f9fff9f960e Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Wed, 6 Jun 2018 16:46:39 -0700 Subject: Fix bug where reset account would not work. Fixes #4462 Ensures that resetAccount() can work on non-stock providers. I'm unclear how this was ever working, this code hasn't moved in months, but users report it recently breaking. Maybe we only recently pushed it to prod. --- app/scripts/controllers/network/network.js | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) (limited to 'app/scripts/controllers') diff --git a/app/scripts/controllers/network/network.js b/app/scripts/controllers/network/network.js index 93fde7c57..5e0c63e7d 100644 --- a/app/scripts/controllers/network/network.js +++ b/app/scripts/controllers/network/network.js @@ -89,14 +89,21 @@ module.exports = class NetworkController extends EventEmitter { type: 'rpc', rpcTarget, } - this.providerStore.updateState(providerConfig) - this._switchNetwork(providerConfig) + this.providerConfig = providerConfig } async setProviderType (type) { assert.notEqual(type, 'rpc', `NetworkController - cannot call "setProviderType" with type 'rpc'. use "setRpcTarget"`) assert(INFURA_PROVIDER_TYPES.includes(type) || type === LOCALHOST, `NetworkController - Unknown rpc type "${type}"`) const providerConfig = { type } + this.providerConfig = providerConfig + } + + resetConnection () { + this.providerConfig = this.getProviderConfig() + } + + set providerConfig (providerConfig) { this.providerStore.updateState(providerConfig) this._switchNetwork(providerConfig) } -- cgit v1.2.3 From 8f93e341750b99b8ff0e66f2a6831799c7d9ab58 Mon Sep 17 00:00:00 2001 From: kumavis Date: Tue, 12 Jun 2018 10:55:54 -0700 Subject: nonce-tracker - wrap nonce calculations in try-catch and release lock on error --- .../controllers/transactions/nonce-tracker.js | 50 ++++++++++++---------- 1 file changed, 28 insertions(+), 22 deletions(-) (limited to 'app/scripts/controllers') diff --git a/app/scripts/controllers/transactions/nonce-tracker.js b/app/scripts/controllers/transactions/nonce-tracker.js index f8cdc5523..ca340bae4 100644 --- a/app/scripts/controllers/transactions/nonce-tracker.js +++ b/app/scripts/controllers/transactions/nonce-tracker.js @@ -49,29 +49,35 @@ class NonceTracker { await this._globalMutexFree() // await lock free, then take lock const releaseLock = await this._takeMutex(address) - // evaluate multiple nextNonce strategies - const nonceDetails = {} - const networkNonceResult = await this._getNetworkNextNonce(address) - const highestLocallyConfirmed = this._getHighestLocallyConfirmed(address) - const nextNetworkNonce = networkNonceResult.nonce - const highestSuggested = Math.max(nextNetworkNonce, highestLocallyConfirmed) - - const pendingTxs = this.getPendingTransactions(address) - const localNonceResult = this._getHighestContinuousFrom(pendingTxs, highestSuggested) || 0 - - nonceDetails.params = { - highestLocallyConfirmed, - highestSuggested, - nextNetworkNonce, + try { + // evaluate multiple nextNonce strategies + const nonceDetails = {} + const networkNonceResult = await this._getNetworkNextNonce(address) + const highestLocallyConfirmed = this._getHighestLocallyConfirmed(address) + const nextNetworkNonce = networkNonceResult.nonce + const highestSuggested = Math.max(nextNetworkNonce, highestLocallyConfirmed) + + const pendingTxs = this.getPendingTransactions(address) + const localNonceResult = this._getHighestContinuousFrom(pendingTxs, highestSuggested) || 0 + + nonceDetails.params = { + highestLocallyConfirmed, + highestSuggested, + nextNetworkNonce, + } + nonceDetails.local = localNonceResult + nonceDetails.network = networkNonceResult + + const nextNonce = Math.max(networkNonceResult.nonce, localNonceResult.nonce) + assert(Number.isInteger(nextNonce), `nonce-tracker - nextNonce is not an integer - got: (${typeof nextNonce}) "${nextNonce}"`) + + // return nonce and release cb + return { nextNonce, nonceDetails, releaseLock } + } catch (err) { + // release lock if we encounter an error + releaseLock() + throw err } - nonceDetails.local = localNonceResult - nonceDetails.network = networkNonceResult - - const nextNonce = Math.max(networkNonceResult.nonce, localNonceResult.nonce) - assert(Number.isInteger(nextNonce), `nonce-tracker - nextNonce is not an integer - got: (${typeof nextNonce}) "${nextNonce}"`) - - // return nonce and release cb - return { nextNonce, nonceDetails, releaseLock } } async _getCurrentBlock () { -- cgit v1.2.3 From 177cc3f280f26c5fb4cfc1b934e95b9d16def1a6 Mon Sep 17 00:00:00 2001 From: kumavis Date: Tue, 12 Jun 2018 11:51:35 -0700 Subject: metamask - ensure all nonce locks are released --- app/scripts/controllers/transactions/index.js | 7 ++++++- app/scripts/controllers/transactions/nonce-tracker.js | 4 ++-- app/scripts/controllers/transactions/pending-tx-tracker.js | 4 ++-- 3 files changed, 10 insertions(+), 5 deletions(-) (limited to 'app/scripts/controllers') diff --git a/app/scripts/controllers/transactions/index.js b/app/scripts/controllers/transactions/index.js index b53947e27..339052543 100644 --- a/app/scripts/controllers/transactions/index.js +++ b/app/scripts/controllers/transactions/index.js @@ -264,7 +264,12 @@ class TransactionController extends EventEmitter { // must set transaction to submitted/failed before releasing lock nonceLock.releaseLock() } catch (err) { - this.txStateManager.setTxStatusFailed(txId, err) + // this is try-catch wrapped so that we can guarantee that the nonceLock is released + try { + this.txStateManager.setTxStatusFailed(txId, err) + } catch (err) { + console.error(err) + } // must set transaction to submitted/failed before releasing lock if (nonceLock) nonceLock.releaseLock() // continue with error chain diff --git a/app/scripts/controllers/transactions/nonce-tracker.js b/app/scripts/controllers/transactions/nonce-tracker.js index ca340bae4..35ca08d6c 100644 --- a/app/scripts/controllers/transactions/nonce-tracker.js +++ b/app/scripts/controllers/transactions/nonce-tracker.js @@ -91,8 +91,8 @@ class NonceTracker { async _globalMutexFree () { const globalMutex = this._lookupMutex('global') - const release = await globalMutex.acquire() - release() + const releaseLock = await globalMutex.acquire() + releaseLock() } async _takeMutex (lockId) { diff --git a/app/scripts/controllers/transactions/pending-tx-tracker.js b/app/scripts/controllers/transactions/pending-tx-tracker.js index 6e2fcb40b..4e41cdaf8 100644 --- a/app/scripts/controllers/transactions/pending-tx-tracker.js +++ b/app/scripts/controllers/transactions/pending-tx-tracker.js @@ -196,14 +196,14 @@ class PendingTransactionTracker extends EventEmitter { async _checkPendingTxs () { const signedTxList = this.getPendingTransactions() // in order to keep the nonceTracker accurate we block it while updating pending transactions - const nonceGlobalLock = await this.nonceTracker.getGlobalLock() + const { releaseLock } = await this.nonceTracker.getGlobalLock() try { await Promise.all(signedTxList.map((txMeta) => this._checkPendingTx(txMeta))) } catch (err) { log.error('PendingTransactionWatcher - Error updating pending transactions') log.error(err) } - nonceGlobalLock.releaseLock() + releaseLock() } /** -- cgit v1.2.3 From 604289c96cde7e5f4634fe5e76a50dfa9174fcbd Mon Sep 17 00:00:00 2001 From: kumavis Date: Tue, 12 Jun 2018 12:08:06 -0700 Subject: controllers - transaction - prefer log over console --- app/scripts/controllers/transactions/index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'app/scripts/controllers') diff --git a/app/scripts/controllers/transactions/index.js b/app/scripts/controllers/transactions/index.js index 339052543..8e2288aed 100644 --- a/app/scripts/controllers/transactions/index.js +++ b/app/scripts/controllers/transactions/index.js @@ -165,7 +165,7 @@ class TransactionController extends EventEmitter { // add default tx params txMeta = await this.addTxGasDefaults(txMeta) } catch (error) { - console.log(error) + log.warn(error) this.txStateManager.setTxStatusFailed(txMeta.id, error) throw error } @@ -268,7 +268,7 @@ class TransactionController extends EventEmitter { try { this.txStateManager.setTxStatusFailed(txId, err) } catch (err) { - console.error(err) + log.error(err) } // must set transaction to submitted/failed before releasing lock if (nonceLock) nonceLock.releaseLock() -- cgit v1.2.3 From a42299aab7d775f8a55d7c5e7fc02192371a3f25 Mon Sep 17 00:00:00 2001 From: Dan Finlay <542863+danfinlay@users.noreply.github.com> Date: Fri, 15 Jun 2018 08:55:39 -0700 Subject: Add apparent phishing address to block list In [this reddit post](https://www.reddit.com/r/Metamask/comments/8r3nsu/help_me_please_somebody_stole_my_ethers/) a user suggests they got some ether stolen after visiting IDEX. Their ether was sent to [this address](https://etherscan.io/address/0x9bcb0a9d99d815bb87ee3191b1399b1bcc46dc77), which is full of comments of people telling similar stories of being phished on IDEX. I think we can safely block this, and probably safe some people some money. --- app/scripts/controllers/transactions/lib/recipient-blacklist-config.json | 1 + 1 file changed, 1 insertion(+) (limited to 'app/scripts/controllers') diff --git a/app/scripts/controllers/transactions/lib/recipient-blacklist-config.json b/app/scripts/controllers/transactions/lib/recipient-blacklist-config.json index b348eb72e..995ca102a 100644 --- a/app/scripts/controllers/transactions/lib/recipient-blacklist-config.json +++ b/app/scripts/controllers/transactions/lib/recipient-blacklist-config.json @@ -1,5 +1,6 @@ { "blacklist": [ + "0x9bcb0A9d99d815Bb87ee3191b1399b1Bcc46dc77", "0x627306090abab3a6e1400e9345bc60c78a8bef57", "0xf17f52151ebef6c7334fad080c5704d77216b732", "0xc5fdf4076b8f3a5357c5e395ab970b5b54098fef", -- cgit v1.2.3 From 753743e7462285e3bffbf1ce53817731da176514 Mon Sep 17 00:00:00 2001 From: kumavis Date: Fri, 15 Jun 2018 10:32:09 -0700 Subject: Update recipient-blacklist-config.json --- .../controllers/transactions/lib/recipient-blacklist-config.json | 2 ++ 1 file changed, 2 insertions(+) (limited to 'app/scripts/controllers') diff --git a/app/scripts/controllers/transactions/lib/recipient-blacklist-config.json b/app/scripts/controllers/transactions/lib/recipient-blacklist-config.json index 995ca102a..f0b427fe3 100644 --- a/app/scripts/controllers/transactions/lib/recipient-blacklist-config.json +++ b/app/scripts/controllers/transactions/lib/recipient-blacklist-config.json @@ -1,6 +1,8 @@ { "blacklist": [ + // IDEX phisher "0x9bcb0A9d99d815Bb87ee3191b1399b1Bcc46dc77", + // Ganache default seed phrases "0x627306090abab3a6e1400e9345bc60c78a8bef57", "0xf17f52151ebef6c7334fad080c5704d77216b732", "0xc5fdf4076b8f3a5357c5e395ab970b5b54098fef", -- cgit v1.2.3 From 83c02f90cf1f85925fc8b7dd242a4cc96c2d509c Mon Sep 17 00:00:00 2001 From: kumavis Date: Fri, 15 Jun 2018 10:47:42 -0700 Subject: blacklist - recipient blacklist as js for inline comments --- .../transactions/lib/recipient-blacklist-checker.js | 2 +- .../transactions/lib/recipient-blacklist-config.json | 17 ----------------- .../controllers/transactions/lib/recipient-blacklist.js | 17 +++++++++++++++++ 3 files changed, 18 insertions(+), 18 deletions(-) delete mode 100644 app/scripts/controllers/transactions/lib/recipient-blacklist-config.json create mode 100644 app/scripts/controllers/transactions/lib/recipient-blacklist.js (limited to 'app/scripts/controllers') diff --git a/app/scripts/controllers/transactions/lib/recipient-blacklist-checker.js b/app/scripts/controllers/transactions/lib/recipient-blacklist-checker.js index 84c6df1f0..e4df2367e 100644 --- a/app/scripts/controllers/transactions/lib/recipient-blacklist-checker.js +++ b/app/scripts/controllers/transactions/lib/recipient-blacklist-checker.js @@ -1,4 +1,4 @@ -const Config = require('./recipient-blacklist-config.json') +const Config = require('./recipient-blacklist.js') /** @module*/ module.exports = { diff --git a/app/scripts/controllers/transactions/lib/recipient-blacklist-config.json b/app/scripts/controllers/transactions/lib/recipient-blacklist-config.json deleted file mode 100644 index f0b427fe3..000000000 --- a/app/scripts/controllers/transactions/lib/recipient-blacklist-config.json +++ /dev/null @@ -1,17 +0,0 @@ -{ - "blacklist": [ - // IDEX phisher - "0x9bcb0A9d99d815Bb87ee3191b1399b1Bcc46dc77", - // Ganache default seed phrases - "0x627306090abab3a6e1400e9345bc60c78a8bef57", - "0xf17f52151ebef6c7334fad080c5704d77216b732", - "0xc5fdf4076b8f3a5357c5e395ab970b5b54098fef", - "0x821aea9a577a9b44299b9c15c88cf3087f3b5544", - "0x0d1d4e623d10f9fba5db95830f7d3839406c6af2", - "0x2932b7a2355d6fecc4b5c0b6bd44cc31df247a2e", - "0x2191ef87e392377ec08e7c08eb105ef5448eced5", - "0x0f4f2ac550a1b4e2280d04c21cea7ebd822934b5", - "0x6330a553fc93768f612722bb8c2ec78ac90b3bbc", - "0x5aeda56215b167893e80b4fe645ba6d5bab767de" - ] -} diff --git a/app/scripts/controllers/transactions/lib/recipient-blacklist.js b/app/scripts/controllers/transactions/lib/recipient-blacklist.js new file mode 100644 index 000000000..810112977 --- /dev/null +++ b/app/scripts/controllers/transactions/lib/recipient-blacklist.js @@ -0,0 +1,17 @@ +module.exports = { + "blacklist": [ + // IDEX phisher + "0x9bcb0A9d99d815Bb87ee3191b1399b1Bcc46dc77", + // Ganache default seed phrases + "0x627306090abab3a6e1400e9345bc60c78a8bef57", + "0xf17f52151ebef6c7334fad080c5704d77216b732", + "0xc5fdf4076b8f3a5357c5e395ab970b5b54098fef", + "0x821aea9a577a9b44299b9c15c88cf3087f3b5544", + "0x0d1d4e623d10f9fba5db95830f7d3839406c6af2", + "0x2932b7a2355d6fecc4b5c0b6bd44cc31df247a2e", + "0x2191ef87e392377ec08e7c08eb105ef5448eced5", + "0x0f4f2ac550a1b4e2280d04c21cea7ebd822934b5", + "0x6330a553fc93768f612722bb8c2ec78ac90b3bbc", + "0x5aeda56215b167893e80b4fe645ba6d5bab767de" + ] +} -- cgit v1.2.3 From 33cb0a8cb2a7fe594eb5f0761e1c7b329a6021d5 Mon Sep 17 00:00:00 2001 From: kumavis Date: Fri, 15 Jun 2018 11:07:56 -0700 Subject: lint - fix recipient-blacklist.js --- .../transactions/lib/recipient-blacklist.js | 26 +++++++++++----------- 1 file changed, 13 insertions(+), 13 deletions(-) (limited to 'app/scripts/controllers') diff --git a/app/scripts/controllers/transactions/lib/recipient-blacklist.js b/app/scripts/controllers/transactions/lib/recipient-blacklist.js index 810112977..08e1a2ccd 100644 --- a/app/scripts/controllers/transactions/lib/recipient-blacklist.js +++ b/app/scripts/controllers/transactions/lib/recipient-blacklist.js @@ -1,17 +1,17 @@ module.exports = { - "blacklist": [ + 'blacklist': [ // IDEX phisher - "0x9bcb0A9d99d815Bb87ee3191b1399b1Bcc46dc77", + '0x9bcb0A9d99d815Bb87ee3191b1399b1Bcc46dc77', // Ganache default seed phrases - "0x627306090abab3a6e1400e9345bc60c78a8bef57", - "0xf17f52151ebef6c7334fad080c5704d77216b732", - "0xc5fdf4076b8f3a5357c5e395ab970b5b54098fef", - "0x821aea9a577a9b44299b9c15c88cf3087f3b5544", - "0x0d1d4e623d10f9fba5db95830f7d3839406c6af2", - "0x2932b7a2355d6fecc4b5c0b6bd44cc31df247a2e", - "0x2191ef87e392377ec08e7c08eb105ef5448eced5", - "0x0f4f2ac550a1b4e2280d04c21cea7ebd822934b5", - "0x6330a553fc93768f612722bb8c2ec78ac90b3bbc", - "0x5aeda56215b167893e80b4fe645ba6d5bab767de" - ] + '0x627306090abab3a6e1400e9345bc60c78a8bef57', + '0xf17f52151ebef6c7334fad080c5704d77216b732', + '0xc5fdf4076b8f3a5357c5e395ab970b5b54098fef', + '0x821aea9a577a9b44299b9c15c88cf3087f3b5544', + '0x0d1d4e623d10f9fba5db95830f7d3839406c6af2', + '0x2932b7a2355d6fecc4b5c0b6bd44cc31df247a2e', + '0x2191ef87e392377ec08e7c08eb105ef5448eced5', + '0x0f4f2ac550a1b4e2280d04c21cea7ebd822934b5', + '0x6330a553fc93768f612722bb8c2ec78ac90b3bbc', + '0x5aeda56215b167893e80b4fe645ba6d5bab767de', + ], } -- cgit v1.2.3