From 1f0223d0a0b617ffaf1704210b7ed328d12c48d1 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Wed, 23 Aug 2017 19:34:20 -0700 Subject: Simplify nonce calculation --- app/scripts/lib/nonce-tracker.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'app/scripts/lib') diff --git a/app/scripts/lib/nonce-tracker.js b/app/scripts/lib/nonce-tracker.js index 30c59fa46..db5a5327f 100644 --- a/app/scripts/lib/nonce-tracker.js +++ b/app/scripts/lib/nonce-tracker.js @@ -28,9 +28,9 @@ class NonceTracker { const releaseLock = await this._takeMutex(address) // evaluate multiple nextNonce strategies const nonceDetails = {} - const localNonceResult = await this._getlocalNextNonce(address) - nonceDetails.local = localNonceResult.details const networkNonceResult = await this._getNetworkNextNonce(address) + const localNonceResult = await this._getLocalNextNonce(address, networkNonceResult) + nonceDetails.local = localNonceResult.details nonceDetails.network = networkNonceResult.details const nextNonce = Math.max(networkNonceResult.nonce, localNonceResult.nonce) assert(Number.isInteger(nextNonce), `nonce-tracker - nextNonce is not an integer - got: (${typeof nextNonce}) "${nextNonce}"`) @@ -81,15 +81,16 @@ class NonceTracker { return { name: 'network', nonce: baseCount, details: nonceDetails } } - async _getlocalNextNonce (address) { + async _getLocalNextNonce (address, networkNonce) { let nextNonce // check our local tx history for the highest nonce (if any) const confirmedTransactions = this.getConfirmedTransactions(address) const pendingTransactions = this.getPendingTransactions(address) const transactions = confirmedTransactions.concat(pendingTransactions) + const highestConfirmedNonce = this._getHighestNonce(confirmedTransactions) const highestPendingNonce = this._getHighestNonce(pendingTransactions) - const highestNonce = this._getHighestNonce(transactions) + const highestNonce = Math.max(highestConfirmedNonce, highestPendingNonce) const haveHighestNonce = Number.isInteger(highestNonce) if (haveHighestNonce) { -- cgit From c4ab7a57799167904b863b9cb9423885a9c1cc66 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Wed, 23 Aug 2017 19:35:49 -0700 Subject: Linted --- app/scripts/lib/nonce-tracker.js | 1 - 1 file changed, 1 deletion(-) (limited to 'app/scripts/lib') diff --git a/app/scripts/lib/nonce-tracker.js b/app/scripts/lib/nonce-tracker.js index db5a5327f..d3c76c230 100644 --- a/app/scripts/lib/nonce-tracker.js +++ b/app/scripts/lib/nonce-tracker.js @@ -86,7 +86,6 @@ class NonceTracker { // check our local tx history for the highest nonce (if any) const confirmedTransactions = this.getConfirmedTransactions(address) const pendingTransactions = this.getPendingTransactions(address) - const transactions = confirmedTransactions.concat(pendingTransactions) const highestConfirmedNonce = this._getHighestNonce(confirmedTransactions) const highestPendingNonce = this._getHighestNonce(pendingTransactions) -- cgit From 221575a191a0a8b8c4c17465a0530561e1905297 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Wed, 23 Aug 2017 20:04:03 -0700 Subject: Fix new test, break an older maybe wrong one --- app/scripts/lib/nonce-tracker.js | 50 +++++++++++++++++++--------------------- 1 file changed, 24 insertions(+), 26 deletions(-) (limited to 'app/scripts/lib') diff --git a/app/scripts/lib/nonce-tracker.js b/app/scripts/lib/nonce-tracker.js index d3c76c230..4f2473cf5 100644 --- a/app/scripts/lib/nonce-tracker.js +++ b/app/scripts/lib/nonce-tracker.js @@ -29,11 +29,16 @@ class NonceTracker { // evaluate multiple nextNonce strategies const nonceDetails = {} const networkNonceResult = await this._getNetworkNextNonce(address) - const localNonceResult = await this._getLocalNextNonce(address, networkNonceResult) + const highestLocallyConfirmed = this._getHighestLocallyConfirmed(address) + const highestConfirmed = Math.max(networkNonceResult.nonce, highestLocallyConfirmed) + const pendingTxs = this.getPendingTransactions(address) + const localNonceResult = this._getHighestContinuousFrom(pendingTxs, highestConfirmed) || 0 + nonceDetails.local = localNonceResult.details nonceDetails.network = networkNonceResult.details 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 } } @@ -77,35 +82,14 @@ class NonceTracker { const baseCountHex = await this.ethQuery.getTransactionCount(address, blockNumber) const baseCount = parseInt(baseCountHex, 16) assert(Number.isInteger(baseCount), `nonce-tracker - baseCount is not an integer - got: (${typeof baseCount}) "${baseCount}"`) - const nonceDetails = { blockNumber, baseCountHex, baseCount } + const nonceDetails = { blockNumber, baseCount } return { name: 'network', nonce: baseCount, details: nonceDetails } } - async _getLocalNextNonce (address, networkNonce) { - let nextNonce - // check our local tx history for the highest nonce (if any) + _getHighestLocallyConfirmed (address) { const confirmedTransactions = this.getConfirmedTransactions(address) - const pendingTransactions = this.getPendingTransactions(address) - - const highestConfirmedNonce = this._getHighestNonce(confirmedTransactions) - const highestPendingNonce = this._getHighestNonce(pendingTransactions) - const highestNonce = Math.max(highestConfirmedNonce, highestPendingNonce) - - const haveHighestNonce = Number.isInteger(highestNonce) - if (haveHighestNonce) { - // next nonce is the nonce after our last - nextNonce = highestNonce + 1 - } else { - // no local tx history so next must be first (zero) - nextNonce = 0 - } - const nonceDetails = { highestNonce, haveHighestNonce, highestConfirmedNonce, highestPendingNonce } - return { name: 'local', nonce: nextNonce, details: nonceDetails } - } - - _getPendingTransactionCount (address) { - const pendingTransactions = this.getPendingTransactions(address) - return this._reduceTxListToUniqueNonces(pendingTransactions).length + const highest = this._getHighestNonce(confirmedTransactions) + return highest } _reduceTxListToUniqueNonces (txList) { @@ -127,6 +111,20 @@ class NonceTracker { return highestNonce } + _getHighestContinuousFrom (txList, startPoint) { + const nonces = txList.map((txMeta) => parseInt(txMeta.txParams.nonce, 16)) + + let highest = startPoint + while (nonces.includes(highest + 1)) { + highest++ + } + + const haveHighestNonce = Number.isInteger(highest) && highest > 0 + const nonce = haveHighestNonce ? highest + 1 : 0 + + return { name: 'local', nonce } + } + // this is a hotfix for the fact that the blockTracker will // change when the network changes _getBlockTracker () { -- cgit From 04d40b114dd12237710f605fe2f0a5f2c337d2cb Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Wed, 23 Aug 2017 20:11:37 -0700 Subject: Got all tests but one passing --- app/scripts/lib/nonce-tracker.js | 1 + 1 file changed, 1 insertion(+) (limited to 'app/scripts/lib') diff --git a/app/scripts/lib/nonce-tracker.js b/app/scripts/lib/nonce-tracker.js index 4f2473cf5..6fcd716f2 100644 --- a/app/scripts/lib/nonce-tracker.js +++ b/app/scripts/lib/nonce-tracker.js @@ -30,6 +30,7 @@ class NonceTracker { const nonceDetails = {} const networkNonceResult = await this._getNetworkNextNonce(address) const highestLocallyConfirmed = this._getHighestLocallyConfirmed(address) + const highestConfirmed = Math.max(networkNonceResult.nonce, highestLocallyConfirmed) const pendingTxs = this.getPendingTransactions(address) const localNonceResult = this._getHighestContinuousFrom(pendingTxs, highestConfirmed) || 0 -- cgit From 855f4eeacbcf7b3e056cf7956edea2c84fa256d5 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Wed, 23 Aug 2017 20:31:03 -0700 Subject: Pass nonce tests --- app/scripts/lib/nonce-tracker.js | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) (limited to 'app/scripts/lib') diff --git a/app/scripts/lib/nonce-tracker.js b/app/scripts/lib/nonce-tracker.js index 6fcd716f2..8c2568c3f 100644 --- a/app/scripts/lib/nonce-tracker.js +++ b/app/scripts/lib/nonce-tracker.js @@ -30,10 +30,12 @@ class NonceTracker { const nonceDetails = {} const networkNonceResult = await this._getNetworkNextNonce(address) const highestLocallyConfirmed = this._getHighestLocallyConfirmed(address) + const nextNetworkNonce = networkNonceResult.nonce + const highestLocalNonce = highestLocallyConfirmed + const highestSuggested = Math.max(nextNetworkNonce, highestLocalNonce) - const highestConfirmed = Math.max(networkNonceResult.nonce, highestLocallyConfirmed) const pendingTxs = this.getPendingTransactions(address) - const localNonceResult = this._getHighestContinuousFrom(pendingTxs, highestConfirmed) || 0 + const localNonceResult = this._getHighestContinuousFrom(pendingTxs, highestSuggested) || 0 nonceDetails.local = localNonceResult.details nonceDetails.network = networkNonceResult.details @@ -90,7 +92,7 @@ class NonceTracker { _getHighestLocallyConfirmed (address) { const confirmedTransactions = this.getConfirmedTransactions(address) const highest = this._getHighestNonce(confirmedTransactions) - return highest + return Number.isInteger(highest) ? highest + 1 : 0 } _reduceTxListToUniqueNonces (txList) { @@ -116,14 +118,11 @@ class NonceTracker { const nonces = txList.map((txMeta) => parseInt(txMeta.txParams.nonce, 16)) let highest = startPoint - while (nonces.includes(highest + 1)) { + while (nonces.includes(highest)) { highest++ } - const haveHighestNonce = Number.isInteger(highest) && highest > 0 - const nonce = haveHighestNonce ? highest + 1 : 0 - - return { name: 'local', nonce } + return { name: 'local', nonce: highest, details: { startPoint, highest } } } // this is a hotfix for the fact that the blockTracker will -- cgit From 55c1a259b1ac01e8a8f6b241bac7a3b1cd16f3ec Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Wed, 23 Aug 2017 21:14:46 -0700 Subject: Fix network nonce parsing --- app/scripts/lib/nonce-tracker.js | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) (limited to 'app/scripts/lib') diff --git a/app/scripts/lib/nonce-tracker.js b/app/scripts/lib/nonce-tracker.js index 8c2568c3f..d49c7f205 100644 --- a/app/scripts/lib/nonce-tracker.js +++ b/app/scripts/lib/nonce-tracker.js @@ -37,8 +37,14 @@ class NonceTracker { const pendingTxs = this.getPendingTransactions(address) const localNonceResult = this._getHighestContinuousFrom(pendingTxs, highestSuggested) || 0 - nonceDetails.local = localNonceResult.details - nonceDetails.network = networkNonceResult.details + nonceDetails.params = { + highestLocalNonce, + 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}"`) @@ -82,8 +88,9 @@ class NonceTracker { // and pending count are from the same block const currentBlock = await this._getCurrentBlock() const blockNumber = currentBlock.blockNumber - const baseCountHex = await this.ethQuery.getTransactionCount(address, blockNumber) - const baseCount = parseInt(baseCountHex, 16) + const baseCountBN = await this.ethQuery.getTransactionCount(address, blockNumber) + const baseString = baseCountBN.toString() + const baseCount = parseInt(baseString) 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 } -- cgit From a122ec1f8ba0935d26a45ce0b26be991d222aaad Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Wed, 23 Aug 2017 21:37:07 -0700 Subject: Use toNumber method --- app/scripts/lib/nonce-tracker.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'app/scripts/lib') diff --git a/app/scripts/lib/nonce-tracker.js b/app/scripts/lib/nonce-tracker.js index d49c7f205..e0e065d82 100644 --- a/app/scripts/lib/nonce-tracker.js +++ b/app/scripts/lib/nonce-tracker.js @@ -89,8 +89,7 @@ class NonceTracker { const currentBlock = await this._getCurrentBlock() const blockNumber = currentBlock.blockNumber const baseCountBN = await this.ethQuery.getTransactionCount(address, blockNumber) - const baseString = baseCountBN.toString() - const baseCount = parseInt(baseString) + 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 } -- cgit From c620123fab9e1eac8d3038204c9cfb04ca85afb1 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Wed, 23 Aug 2017 21:50:28 -0700 Subject: Enforce nonces as type string --- app/scripts/lib/nonce-tracker.js | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) (limited to 'app/scripts/lib') diff --git a/app/scripts/lib/nonce-tracker.js b/app/scripts/lib/nonce-tracker.js index e0e065d82..08f1e1e86 100644 --- a/app/scripts/lib/nonce-tracker.js +++ b/app/scripts/lib/nonce-tracker.js @@ -115,13 +115,21 @@ class NonceTracker { } _getHighestNonce (txList) { - const nonces = txList.map((txMeta) => parseInt(txMeta.txParams.nonce, 16)) + 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 } _getHighestContinuousFrom (txList, startPoint) { - const nonces = txList.map((txMeta) => parseInt(txMeta.txParams.nonce, 16)) + 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)) { -- cgit