diff options
| -rw-r--r-- | app/scripts/controllers/transactions/index.js | 2 | ||||
| -rw-r--r-- | app/scripts/controllers/transactions/nonce-tracker.js | 161 | ||||
| -rw-r--r-- | package-lock.json | 79 | ||||
| -rw-r--r-- | package.json | 1 | ||||
| -rw-r--r-- | test/unit/app/controllers/transactions/nonce-tracker-test.js | 238 |
5 files changed, 74 insertions, 407 deletions
diff --git a/app/scripts/controllers/transactions/index.js b/app/scripts/controllers/transactions/index.js index dd497a11e..1ae925835 100644 --- a/app/scripts/controllers/transactions/index.js +++ b/app/scripts/controllers/transactions/index.js @@ -17,7 +17,7 @@ const { const TransactionStateManager = require('./tx-state-manager') const TxGasUtil = require('./tx-gas-utils') const PendingTransactionTracker = require('./pending-tx-tracker') -const NonceTracker = require('./nonce-tracker') +const NonceTracker = require('nonce-tracker') const txUtils = require('./lib/util') const cleanErrorStack = require('../../lib/cleanErrorStack') const log = require('loglevel') diff --git a/app/scripts/controllers/transactions/nonce-tracker.js b/app/scripts/controllers/transactions/nonce-tracker.js deleted file mode 100644 index 421036368..000000000 --- a/app/scripts/controllers/transactions/nonce-tracker.js +++ /dev/null @@ -1,161 +0,0 @@ -const EthQuery = require('ethjs-query') -const assert = require('assert') -const Mutex = require('await-semaphore').Mutex -/** - @param opts {Object} - @param {Object} opts.provider a ethereum provider - @param {Function} opts.getPendingTransactions a function that returns an array of txMeta - whosee status is `submitted` - @param {Function} opts.getConfirmedTransactions a function that returns an array of txMeta - whose status is `confirmed` - @class -*/ -class NonceTracker { - - constructor ({ provider, blockTracker, getPendingTransactions, getConfirmedTransactions }) { - this.provider = provider - this.blockTracker = blockTracker - this.ethQuery = new EthQuery(provider) - this.getPendingTransactions = getPendingTransactions - this.getConfirmedTransactions = getConfirmedTransactions - this.lockMap = {} - } - - /** - @returns {Promise<Object>} with the key releaseLock (the gloabl mutex) - */ - async getGlobalLock () { - const globalMutex = this._lookupMutex('global') - // await global mutex free - const releaseLock = await globalMutex.acquire() - return { releaseLock } - } - - /** - * @typedef NonceDetails - * @property {number} highestLocallyConfirmed - A hex string of the highest nonce on a confirmed transaction. - * @property {number} nextNetworkNonce - The next nonce suggested by the eth_getTransactionCount method. - * @property {number} highestSuggested - The maximum between the other two, the number returned. - */ - - /** - this will return an object with the `nextNonce` `nonceDetails` of type NonceDetails, and the releaseLock - Note: releaseLock must be called after adding a signed tx to pending transactions (or discarding). - - @param address {string} the hex string for the address whose nonce we are calculating - @returns {Promise<NonceDetails>} - */ - async getNonceLock (address) { - // await global mutex free - await this._globalMutexFree() - // await lock free, then take lock - const releaseLock = await this._takeMutex(address) - 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 - } - } - - async _globalMutexFree () { - const globalMutex = this._lookupMutex('global') - const releaseLock = await globalMutex.acquire() - releaseLock() - } - - 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 - } - - async _getNetworkNextNonce (address) { - // calculate next nonce - // we need to make sure our base count - // and pending count are from the same block - const blockNumber = await this.blockTracker.getLatestBlock() - const baseCountBN = await this.ethQuery.getTransactionCount(address, blockNumber) - const baseCount = baseCountBN.toNumber() - assert(Number.isInteger(baseCount), `nonce-tracker - baseCount is not an integer - got: (${typeof baseCount}) "${baseCount}"`) - const nonceDetails = { blockNumber, baseCount } - return { name: 'network', nonce: baseCount, details: nonceDetails } - } - - _getHighestLocallyConfirmed (address) { - const confirmedTransactions = this.getConfirmedTransactions(address) - const highest = this._getHighestNonce(confirmedTransactions) - return Number.isInteger(highest) ? highest + 1 : 0 - } - - _getHighestNonce (txList) { - const nonces = txList.map((txMeta) => { - const nonce = txMeta.txParams.nonce - assert(typeof nonce, 'string', 'nonces should be hex strings') - return parseInt(nonce, 16) - }) - const highestNonce = Math.max.apply(null, nonces) - return highestNonce - } - - /** - @typedef {object} highestContinuousFrom - @property {string} - name the name for how the nonce was calculated based on the data used - @property {number} - nonce the next suggested nonce - @property {object} - details the provided starting nonce that was used (for debugging) - */ - /** - @param txList {array} - list of txMeta's - @param startPoint {number} - the highest known locally confirmed nonce - @returns {highestContinuousFrom} - */ - _getHighestContinuousFrom (txList, startPoint) { - const nonces = txList.map((txMeta) => { - const nonce = txMeta.txParams.nonce - assert(typeof nonce, 'string', 'nonces should be hex strings') - return parseInt(nonce, 16) - }) - - let highest = startPoint - while (nonces.includes(highest)) { - highest++ - } - - return { name: 'local', nonce: highest, details: { startPoint, highest } } - } - -} - -module.exports = NonceTracker diff --git a/package-lock.json b/package-lock.json index 8844b21a5..526420356 100644 --- a/package-lock.json +++ b/package-lock.json @@ -8854,7 +8854,7 @@ }, "engine.io-client": { "version": "3.2.1", - "resolved": "http://registry.npmjs.org/engine.io-client/-/engine.io-client-3.2.1.tgz", + "resolved": "https://registry.npmjs.org/engine.io-client/-/engine.io-client-3.2.1.tgz", "integrity": "sha512-y5AbkytWeM4jQr7m/koQLc5AxpRKC1hEVUb/s1FUAWEJq5AzJJ4NLvzuKPuxtDi5Mq755WuDvZ6Iv2rXj4PTzw==", "dev": true, "requires": { @@ -9473,7 +9473,7 @@ }, "inquirer": { "version": "0.12.0", - "resolved": "http://registry.npmjs.org/inquirer/-/inquirer-0.12.0.tgz", + "resolved": "https://registry.npmjs.org/inquirer/-/inquirer-0.12.0.tgz", "integrity": "sha1-HvK/1jUE3wvHV4X/+MLEHfEvB34=", "dev": true, "requires": { @@ -9566,7 +9566,7 @@ }, "table": { "version": "3.8.3", - "resolved": "http://registry.npmjs.org/table/-/table-3.8.3.tgz", + "resolved": "https://registry.npmjs.org/table/-/table-3.8.3.tgz", "integrity": "sha1-K7xULw/amGGnVdOUf+/Ys/UThV8=", "dev": true, "requires": { @@ -10157,7 +10157,7 @@ "dependencies": { "babelify": { "version": "7.3.0", - "resolved": "http://registry.npmjs.org/babelify/-/babelify-7.3.0.tgz", + "resolved": "https://registry.npmjs.org/babelify/-/babelify-7.3.0.tgz", "integrity": "sha1-qlau3nBn/XvVSWZu4W3ChQh+iOU=", "dev": true, "requires": { @@ -15815,6 +15815,23 @@ "requires": { "bn.js": "^4.11.8", "ethereumjs-util": "^6.0.0" + }, + "dependencies": { + "ethereumjs-util": { + "version": "6.1.0", + "resolved": "https://registry.npmjs.org/ethereumjs-util/-/ethereumjs-util-6.1.0.tgz", + "integrity": "sha512-URESKMFbDeJxnAxPppnk2fN6Y3BIatn9fwn76Lm8bQlt+s52TpG8dN9M66MLPuRAiAOIqL3dfwqWJf0sd0fL0Q==", + "dev": true, + "requires": { + "bn.js": "^4.11.0", + "create-hash": "^1.1.2", + "ethjs-util": "0.1.6", + "keccak": "^1.0.2", + "rlp": "^2.0.0", + "safe-buffer": "^5.1.1", + "secp256k1": "^3.0.1" + } + } } }, "ethereumjs-block": { @@ -19798,6 +19815,23 @@ "requires": { "bn.js": "^4.11.8", "ethereumjs-util": "^6.0.0" + }, + "dependencies": { + "ethereumjs-util": { + "version": "6.1.0", + "resolved": "https://registry.npmjs.org/ethereumjs-util/-/ethereumjs-util-6.1.0.tgz", + "integrity": "sha512-URESKMFbDeJxnAxPppnk2fN6Y3BIatn9fwn76Lm8bQlt+s52TpG8dN9M66MLPuRAiAOIqL3dfwqWJf0sd0fL0Q==", + "dev": true, + "requires": { + "bn.js": "^4.11.0", + "create-hash": "^1.1.2", + "ethjs-util": "0.1.6", + "keccak": "^1.0.2", + "rlp": "^2.0.0", + "safe-buffer": "^5.1.1", + "secp256k1": "^3.0.1" + } + } } }, "ethereumjs-block": { @@ -20952,7 +20986,7 @@ }, "got": { "version": "5.6.0", - "resolved": "http://registry.npmjs.org/got/-/got-5.6.0.tgz", + "resolved": "https://registry.npmjs.org/got/-/got-5.6.0.tgz", "integrity": "sha1-ux1+4WO3gIK7yOuDbz85UATqb78=", "dev": true, "requires": { @@ -27909,6 +27943,37 @@ "underscore": "~1.4.4" } }, + "nonce-tracker": { + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/nonce-tracker/-/nonce-tracker-1.0.0.tgz", + "integrity": "sha512-hxKokxgLvOZx9A5qPQKwL34G1/YwMC5xJWZHFUKfvwxypkn2nP0KVJjbcoXwY6pXsRRa11KdFEPW61N4YCGnWQ==", + "requires": { + "assert": "^1.4.1", + "await-semaphore": "^0.1.3", + "ethjs-query": "^0.3.8" + }, + "dependencies": { + "ethjs-query": { + "version": "0.3.8", + "resolved": "https://registry.npmjs.org/ethjs-query/-/ethjs-query-0.3.8.tgz", + "integrity": "sha512-/J5JydqrOzU8O7VBOwZKUWXxHDGr46VqNjBCJgBVNNda+tv7Xc8Y2uJc6aMHHVbeN3YOQ7YRElgIc0q1CI02lQ==", + "requires": { + "babel-runtime": "^6.26.0", + "ethjs-format": "0.2.7", + "ethjs-rpc": "0.2.0", + "promise-to-callback": "^1.0.0" + } + }, + "ethjs-rpc": { + "version": "0.2.0", + "resolved": "https://registry.npmjs.org/ethjs-rpc/-/ethjs-rpc-0.2.0.tgz", + "integrity": "sha512-RINulkNZTKnj4R/cjYYtYMnFFaBcVALzbtEJEONrrka8IeoarNB9Jbzn+2rT00Cv8y/CxAI+GgY1d0/i2iQeOg==", + "requires": { + "promise-to-callback": "^1.0.0" + } + } + } + }, "nopt": { "version": "3.0.6", "resolved": "https://registry.npmjs.org/nopt/-/nopt-3.0.6.tgz", @@ -30423,7 +30488,7 @@ }, "po2json": { "version": "0.4.5", - "resolved": "http://registry.npmjs.org/po2json/-/po2json-0.4.5.tgz", + "resolved": "https://registry.npmjs.org/po2json/-/po2json-0.4.5.tgz", "integrity": "sha1-R7spUtoy1Yob4vJWpZjuvAt0URg=", "dev": true, "requires": { @@ -35856,7 +35921,7 @@ }, "socket.io-parser": { "version": "3.2.0", - "resolved": "http://registry.npmjs.org/socket.io-parser/-/socket.io-parser-3.2.0.tgz", + "resolved": "https://registry.npmjs.org/socket.io-parser/-/socket.io-parser-3.2.0.tgz", "integrity": "sha512-FYiBx7rc/KORMJlgsXysflWx/RIvtqZbyGLlHZvjfmPTPeuD/I8MaW7cfFrj5tRltICJdgwflhfZ3NVVbVLFQA==", "dev": true, "requires": { diff --git a/package.json b/package.json index 0ae402600..89e0388d4 100644 --- a/package.json +++ b/package.json @@ -133,6 +133,7 @@ "mkdirp": "^0.5.1", "multihashes": "^0.4.12", "multiplex": "^6.7.0", + "nonce-tracker": "^1.0.0", "number-to-bn": "^1.7.0", "obj-multiplex": "^1.0.0", "obs-store": "^3.0.2", diff --git a/test/unit/app/controllers/transactions/nonce-tracker-test.js b/test/unit/app/controllers/transactions/nonce-tracker-test.js deleted file mode 100644 index 51ac390e9..000000000 --- a/test/unit/app/controllers/transactions/nonce-tracker-test.js +++ /dev/null @@ -1,238 +0,0 @@ -const assert = require('assert') -const NonceTracker = require('../../../../../app/scripts/controllers/transactions/nonce-tracker') -const MockTxGen = require('../../../../lib/mock-tx-gen') -const providerResultStub = {} - -describe('Nonce Tracker', function () { - let nonceTracker, pendingTxs, confirmedTxs - - describe('#getNonceLock', function () { - - describe('with 3 confirmed and 1 pending', function () { - beforeEach(function () { - const txGen = new MockTxGen() - confirmedTxs = txGen.generate({ status: 'confirmed' }, { count: 3 }) - pendingTxs = txGen.generate({ status: 'submitted' }, { count: 1 }) - nonceTracker = generateNonceTrackerWith(pendingTxs, confirmedTxs, '0x1') - }) - - it('should return 4', async function () { - this.timeout(15000) - const nonceLock = await nonceTracker.getNonceLock('0x7d3517b0d011698406d6e0aed8453f0be2697926') - assert.equal(nonceLock.nextNonce, '4', `nonce should be 4 got ${nonceLock.nextNonce}`) - await nonceLock.releaseLock() - }) - - it('should use localNonce if network returns a nonce lower then a confirmed tx in state', async function () { - this.timeout(15000) - const nonceLock = await nonceTracker.getNonceLock('0x7d3517b0d011698406d6e0aed8453f0be2697926') - assert.equal(nonceLock.nextNonce, '4', 'nonce should be 4') - await nonceLock.releaseLock() - }) - }) - - describe('sentry issue 476304902', function () { - beforeEach(function () { - const txGen = new MockTxGen() - pendingTxs = txGen.generate({ status: 'submitted' }, { - fromNonce: 3, - count: 29, - }) - nonceTracker = generateNonceTrackerWith(pendingTxs, [], '0x3') - }) - - it('should return 9', async function () { - this.timeout(15000) - const nonceLock = await nonceTracker.getNonceLock('0x7d3517b0d011698406d6e0aed8453f0be2697926') - assert.equal(nonceLock.nextNonce, '32', `nonce should be 32 got ${nonceLock.nextNonce}`) - await nonceLock.releaseLock() - }) - }) - - describe('issue 3670', function () { - beforeEach(function () { - const txGen = new MockTxGen() - pendingTxs = txGen.generate({ status: 'submitted' }, { - fromNonce: 6, - count: 3, - }) - nonceTracker = generateNonceTrackerWith(pendingTxs, [], '0x6') - }) - - it('should return 9', async function () { - this.timeout(15000) - const nonceLock = await nonceTracker.getNonceLock('0x7d3517b0d011698406d6e0aed8453f0be2697926') - assert.equal(nonceLock.nextNonce, '9', `nonce should be 9 got ${nonceLock.nextNonce}`) - await nonceLock.releaseLock() - }) - }) - - describe('with no previous txs', function () { - beforeEach(function () { - nonceTracker = generateNonceTrackerWith([], []) - }) - - it('should return 0', async function () { - this.timeout(15000) - const nonceLock = await nonceTracker.getNonceLock('0x7d3517b0d011698406d6e0aed8453f0be2697926') - assert.equal(nonceLock.nextNonce, '0', `nonce should be 0 returned ${nonceLock.nextNonce}`) - await nonceLock.releaseLock() - }) - }) - - describe('with multiple previous txs with same nonce', function () { - beforeEach(function () { - const txGen = new MockTxGen() - confirmedTxs = txGen.generate({ status: 'confirmed' }, { count: 1 }) - pendingTxs = txGen.generate({ - status: 'submitted', - txParams: { nonce: '0x01' }, - }, { count: 5 }) - - nonceTracker = generateNonceTrackerWith(pendingTxs, confirmedTxs, '0x0') - }) - - it('should return nonce after those', async function () { - this.timeout(15000) - const nonceLock = await nonceTracker.getNonceLock('0x7d3517b0d011698406d6e0aed8453f0be2697926') - assert.equal(nonceLock.nextNonce, '2', `nonce should be 2 got ${nonceLock.nextNonce}`) - await nonceLock.releaseLock() - }) - }) - - describe('when local confirmed count is higher than network nonce', function () { - beforeEach(function () { - const txGen = new MockTxGen() - confirmedTxs = txGen.generate({ status: 'confirmed' }, { count: 3 }) - nonceTracker = generateNonceTrackerWith([], confirmedTxs, '0x1') - }) - - it('should return nonce after those', async function () { - this.timeout(15000) - const nonceLock = await nonceTracker.getNonceLock('0x7d3517b0d011698406d6e0aed8453f0be2697926') - assert.equal(nonceLock.nextNonce, '3', `nonce should be 3 got ${nonceLock.nextNonce}`) - await nonceLock.releaseLock() - }) - }) - - describe('when local pending count is higher than other metrics', function () { - beforeEach(function () { - const txGen = new MockTxGen() - pendingTxs = txGen.generate({ status: 'submitted' }, { count: 2 }) - nonceTracker = generateNonceTrackerWith(pendingTxs, []) - }) - - it('should return nonce after those', async function () { - this.timeout(15000) - const nonceLock = await nonceTracker.getNonceLock('0x7d3517b0d011698406d6e0aed8453f0be2697926') - assert.equal(nonceLock.nextNonce, '2', `nonce should be 2 got ${nonceLock.nextNonce}`) - await nonceLock.releaseLock() - }) - }) - - describe('when provider nonce is higher than other metrics', function () { - beforeEach(function () { - const txGen = new MockTxGen() - pendingTxs = txGen.generate({ status: 'submitted' }, { count: 2 }) - nonceTracker = generateNonceTrackerWith(pendingTxs, [], '0x05') - }) - - it('should return nonce after those', async function () { - this.timeout(15000) - const nonceLock = await nonceTracker.getNonceLock('0x7d3517b0d011698406d6e0aed8453f0be2697926') - assert.equal(nonceLock.nextNonce, '5', `nonce should be 5 got ${nonceLock.nextNonce}`) - await nonceLock.releaseLock() - }) - }) - - describe('when there are some pending nonces below the remote one and some over.', function () { - beforeEach(function () { - const txGen = new MockTxGen() - pendingTxs = txGen.generate({ status: 'submitted' }, { count: 5 }) - nonceTracker = generateNonceTrackerWith(pendingTxs, [], '0x03') - }) - - it('should return nonce after those', async function () { - this.timeout(15000) - const nonceLock = await nonceTracker.getNonceLock('0x7d3517b0d011698406d6e0aed8453f0be2697926') - assert.equal(nonceLock.nextNonce, '5', `nonce should be 5 got ${nonceLock.nextNonce}`) - await nonceLock.releaseLock() - }) - }) - - describe('when there are pending nonces non sequentially over the network nonce.', function () { - beforeEach(function () { - const txGen = new MockTxGen() - txGen.generate({ status: 'submitted' }, { count: 5 }) - // 5 over that number - pendingTxs = txGen.generate({ status: 'submitted' }, { count: 5 }) - nonceTracker = generateNonceTrackerWith(pendingTxs, [], '0x00') - }) - - it('should return nonce after network nonce', async function () { - this.timeout(15000) - const nonceLock = await nonceTracker.getNonceLock('0x7d3517b0d011698406d6e0aed8453f0be2697926') - assert.equal(nonceLock.nextNonce, '0', `nonce should be 0 got ${nonceLock.nextNonce}`) - await nonceLock.releaseLock() - }) - }) - - describe('When all three return different values', function () { - beforeEach(function () { - const txGen = new MockTxGen() - confirmedTxs = txGen.generate({ status: 'confirmed' }, { count: 10 }) - pendingTxs = txGen.generate({ - status: 'submitted', - nonce: 100, - }, { count: 1 }) - // 0x32 is 50 in hex: - nonceTracker = generateNonceTrackerWith(pendingTxs, confirmedTxs, '0x32') - }) - - it('should return nonce after network nonce', async function () { - this.timeout(15000) - const nonceLock = await nonceTracker.getNonceLock('0x7d3517b0d011698406d6e0aed8453f0be2697926') - assert.equal(nonceLock.nextNonce, '50', `nonce should be 50 got ${nonceLock.nextNonce}`) - await nonceLock.releaseLock() - }) - }) - - describe('Faq issue 67', function () { - beforeEach(function () { - const txGen = new MockTxGen() - confirmedTxs = txGen.generate({ status: 'confirmed' }, { count: 64 }) - pendingTxs = txGen.generate({ - status: 'submitted', - }, { count: 10 }) - // 0x40 is 64 in hex: - nonceTracker = generateNonceTrackerWith(pendingTxs, [], '0x40') - }) - - it('should return nonce after network nonce', async function () { - this.timeout(15000) - const nonceLock = await nonceTracker.getNonceLock('0x7d3517b0d011698406d6e0aed8453f0be2697926') - assert.equal(nonceLock.nextNonce, '74', `nonce should be 74 got ${nonceLock.nextNonce}`) - await nonceLock.releaseLock() - }) - }) - }) -}) - -function generateNonceTrackerWith (pending, confirmed, providerStub = '0x0') { - const getPendingTransactions = () => pending - const getConfirmedTransactions = () => confirmed - providerResultStub.result = providerStub - const provider = { - sendAsync: (_, cb) => { cb(undefined, providerResultStub) }, - } - const blockTracker = { - getCurrentBlock: () => '0x11b568', - getLatestBlock: async () => '0x11b568', - } - return new NonceTracker({ - provider, - blockTracker, - getPendingTransactions, - getConfirmedTransactions, - }) -} |