aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--CHANGELOG.md1
-rw-r--r--app/scripts/controllers/transactions.js71
-rw-r--r--app/scripts/lib/nonce-tracker.js59
-rw-r--r--package.json1
4 files changed, 88 insertions, 44 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5d15b3512..4d265d318 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -6,6 +6,7 @@
- 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.
## 3.9.0 2017-7-12
diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js
index 61e96ca13..5f3d84ebe 100644
--- a/app/scripts/controllers/transactions.js
+++ b/app/scripts/controllers/transactions.js
@@ -3,6 +3,7 @@ const async = require('async')
const extend = require('xtend')
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 NonceTracker = require('../lib/nonce-tracker')
@@ -481,35 +482,51 @@ module.exports = class TransactionController extends EventEmitter {
// 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/nonce-tracker.js b/app/scripts/lib/nonce-tracker.js
index ab2893b10..b76dac4e8 100644
--- a/app/scripts/lib/nonce-tracker.js
+++ b/app/scripts/lib/nonce-tracker.js
@@ -1,4 +1,6 @@
const EthQuery = require('eth-query')
+const assert = require('assert')
+const Mutex = require('await-semaphore').Mutex
class NonceTracker {
@@ -9,22 +11,34 @@ class NonceTracker {
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 lock free
- await this.lockMap[address]
- // take lock
- const releaseLock = this._takeLock(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 baseCount = await this._getTxCount(address, currentBlock)
- const nextNonce = parseInt(baseCount) + pendingTransactions.length
+ 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: nextNonce.toString(16), releaseLock }
+ return { nextNonce, releaseLock }
}
async _getCurrentBlock () {
@@ -35,16 +49,6 @@ class NonceTracker {
})
}
- _takeLock (lockId) {
- let releaseLock = null
- // create and store lock
- const lock = new Promise((resolve, reject) => { releaseLock = resolve })
- this.lockMap[lockId] = lock
- // setup lock teardown
- lock.then(() => delete this.lockMap[lockId])
- return releaseLock
- }
-
async _getTxCount (address, currentBlock) {
const blockNumber = currentBlock.number
return new Promise((resolve, reject) => {
@@ -54,6 +58,27 @@ class NonceTracker {
})
}
+ 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/package.json b/package.json
index e0bb303bf..d40ad068b 100644
--- a/package.json
+++ b/package.json
@@ -47,6 +47,7 @@
},
"dependencies": {
"async": "^1.5.2",
+ "await-semaphore": "^0.1.1",
"babel-runtime": "^6.23.0",
"bip39": "^2.2.0",
"bluebird": "^3.5.0",