aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--CHANGELOG.md5
-rw-r--r--app/manifest.json2
-rw-r--r--app/scripts/controllers/transactions.js6
-rw-r--r--app/scripts/lib/nonce-tracker.js74
-rw-r--r--app/scripts/lib/pending-tx-tracker.js8
-rw-r--r--app/scripts/migrations/019.js83
-rw-r--r--app/scripts/migrations/index.js1
-rw-r--r--test/unit/nonce-tracker-test.js51
8 files changed, 189 insertions, 41 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 28e7ec92d..a0fe916dd 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,6 +5,11 @@
- Make eth_sign deprecation warning less noisy
- Fix bug with network version serialization over synchronous RPC
+## 3.9.11 2017-8-24
+
+- Fix nonce calculation bug that would sometimes generate very wrong nonces.
+- Give up resubmitting a transaction after 3500 blocks.
+
## 3.9.10 2017-8-23
- Improve nonce calculation, to prevent bug where people are unable to send transactions reliably.
diff --git a/app/manifest.json b/app/manifest.json
index 60eeb2a35..7ec32ea78 100644
--- a/app/manifest.json
+++ b/app/manifest.json
@@ -1,7 +1,7 @@
{
"name": "MetaMask",
"short_name": "Metamask",
- "version": "3.9.10",
+ "version": "3.9.11",
"manifest_version": 2,
"author": "https://metamask.io",
"description": "Ethereum Browser Extension",
diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js
index 6f49c9633..fb3be6073 100644
--- a/app/scripts/controllers/transactions.js
+++ b/app/scripts/controllers/transactions.js
@@ -40,6 +40,10 @@ module.exports = class TransactionController extends EventEmitter {
err: undefined,
})
},
+ giveUpOnTransaction: (txId) => {
+ const msg = `Gave up submitting after 3500 blocks un-mined.`
+ this.setTxStatusFailed(txId, msg)
+ },
})
this.query = new EthQuery(this.provider)
this.txProviderUtil = new TxProviderUtil(this.provider)
@@ -451,4 +455,4 @@ module.exports = class TransactionController extends EventEmitter {
})
this.memStore.updateState({ unapprovedTxs, selectedAddressTxList })
}
-} \ No newline at end of file
+}
diff --git a/app/scripts/lib/nonce-tracker.js b/app/scripts/lib/nonce-tracker.js
index 30c59fa46..0029ac953 100644
--- a/app/scripts/lib/nonce-tracker.js
+++ b/app/scripts/lib/nonce-tracker.js
@@ -28,12 +28,26 @@ 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)
- nonceDetails.network = networkNonceResult.details
+ const highestLocallyConfirmed = this._getHighestLocallyConfirmed(address)
+ const nextNetworkNonce = networkNonceResult.nonce
+ const highestLocalNonce = highestLocallyConfirmed
+ const highestSuggested = Math.max(nextNetworkNonce, highestLocalNonce)
+
+ const pendingTxs = this.getPendingTransactions(address)
+ const localNonceResult = this._getHighestContinuousFrom(pendingTxs, highestSuggested) || 0
+
+ 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}"`)
+
// return nonce and release cb
return { nextNonce, nonceDetails, releaseLock }
}
@@ -74,38 +88,17 @@ 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 || 'latest')
+ const baseCount = baseCountBN.toNumber()
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) {
- 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 transactions = confirmedTransactions.concat(pendingTransactions)
- const highestConfirmedNonce = this._getHighestNonce(confirmedTransactions)
- const highestPendingNonce = this._getHighestNonce(pendingTransactions)
- const highestNonce = this._getHighestNonce(transactions)
-
- 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 Number.isInteger(highest) ? highest + 1 : 0
}
_reduceTxListToUniqueNonces (txList) {
@@ -122,11 +115,30 @@ 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) => {
+ 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 } }
+ }
+
// this is a hotfix for the fact that the blockTracker will
// change when the network changes
_getBlockTracker () {
diff --git a/app/scripts/lib/pending-tx-tracker.js b/app/scripts/lib/pending-tx-tracker.js
index 19720db3f..b90851b58 100644
--- a/app/scripts/lib/pending-tx-tracker.js
+++ b/app/scripts/lib/pending-tx-tracker.js
@@ -1,6 +1,7 @@
const EventEmitter = require('events')
const EthQuery = require('ethjs-query')
const sufficientBalance = require('./util').sufficientBalance
+const RETRY_LIMIT = 3500 // Retry 3500 blocks, or about 1 day.
/*
Utility class for tracking the transactions as they
@@ -28,6 +29,7 @@ module.exports = class PendingTransactionTracker extends EventEmitter {
this.getBalance = config.getBalance
this.getPendingTransactions = config.getPendingTransactions
this.publishTransaction = config.publishTransaction
+ this.giveUpOnTransaction = config.giveUpOnTransaction
}
// checks if a signed tx is in a block and
@@ -100,6 +102,10 @@ module.exports = class PendingTransactionTracker extends EventEmitter {
if (balance === undefined) return
if (!('retryCount' in txMeta)) txMeta.retryCount = 0
+ if (txMeta.retryCount > RETRY_LIMIT) {
+ return this.giveUpOnTransaction(txMeta.id)
+ }
+
// if the value of the transaction is greater then the balance, fail.
if (!sufficientBalance(txMeta.txParams, balance)) {
const insufficientFundsError = new Error('Insufficient balance during rebroadcast.')
@@ -160,4 +166,4 @@ module.exports = class PendingTransactionTracker extends EventEmitter {
}
nonceGlobalLock.releaseLock()
}
-} \ No newline at end of file
+}
diff --git a/app/scripts/migrations/019.js b/app/scripts/migrations/019.js
new file mode 100644
index 000000000..072c96370
--- /dev/null
+++ b/app/scripts/migrations/019.js
@@ -0,0 +1,83 @@
+
+const version = 19
+
+/*
+
+This migration sets transactions as failed
+whos nonce is too high
+
+*/
+
+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, _, txList) => {
+ if (txMeta.status !== 'submitted') return txMeta
+
+ const confirmedTxs = txList.filter((tx) => tx.status === 'confirmed')
+ .filter((tx) => tx.txParams.from === txMeta.txParams.from)
+ .filter((tx) => tx.metamaskNetworkId.from === txMeta.metamaskNetworkId.from)
+ const highestConfirmedNonce = getHighestNonce(confirmedTxs)
+
+ const pendingTxs = txList.filter((tx) => tx.status === 'submitted')
+ .filter((tx) => tx.txParams.from === txMeta.txParams.from)
+ .filter((tx) => tx.metamaskNetworkId.from === txMeta.metamaskNetworkId.from)
+ const highestContinuousNonce = getHighestContinuousFrom(pendingTxs, highestConfirmedNonce)
+
+ const maxNonce = Math.max(highestContinuousNonce, highestConfirmedNonce)
+
+ if (parseInt(txMeta.txParams.nonce, 16) > maxNonce + 1) {
+ txMeta.status = 'failed'
+ txMeta.err = {
+ message: 'nonce too high',
+ note: 'migration 019 custom error',
+ }
+ }
+ return txMeta
+ })
+ return newState
+}
+
+function getHighestContinuousFrom (txList, startPoint) {
+ const nonces = txList.map((txMeta) => {
+ const nonce = txMeta.txParams.nonce
+ return parseInt(nonce, 16)
+ })
+
+ let highest = startPoint
+ while (nonces.includes(highest)) {
+ highest++
+ }
+
+ return highest
+}
+
+function getHighestNonce (txList) {
+ const nonces = txList.map((txMeta) => {
+ const nonce = txMeta.txParams.nonce
+ return parseInt(nonce || '0x0', 16)
+ })
+ const highestNonce = Math.max.apply(null, nonces)
+ return highestNonce
+}
+
diff --git a/app/scripts/migrations/index.js b/app/scripts/migrations/index.js
index 6bfc622f7..e9cbd7b98 100644
--- a/app/scripts/migrations/index.js
+++ b/app/scripts/migrations/index.js
@@ -29,4 +29,5 @@ module.exports = [
require('./016'),
require('./017'),
require('./018'),
+ require('./019'),
]
diff --git a/test/unit/nonce-tracker-test.js b/test/unit/nonce-tracker-test.js
index 11f99751c..3312a3bd0 100644
--- a/test/unit/nonce-tracker-test.js
+++ b/test/unit/nonce-tracker-test.js
@@ -18,10 +18,10 @@ describe('Nonce Tracker', function () {
nonceTracker = generateNonceTrackerWith(pendingTxs, confirmedTxs, '0x1')
})
- it('should work', async function () {
+ it('should return 4', async function () {
this.timeout(15000)
const nonceLock = await nonceTracker.getNonceLock('0x7d3517b0d011698406d6e0aed8453f0be2697926')
- assert.equal(nonceLock.nextNonce, '4', 'nonce should be 4')
+ assert.equal(nonceLock.nextNonce, '4', `nonce should be 4 got ${nonceLock.nextNonce}`)
await nonceLock.releaseLock()
})
@@ -41,7 +41,7 @@ describe('Nonce Tracker', function () {
it('should return 0', async function () {
this.timeout(15000)
const nonceLock = await nonceTracker.getNonceLock('0x7d3517b0d011698406d6e0aed8453f0be2697926')
- assert.equal(nonceLock.nextNonce, '0', 'nonce should be 0')
+ assert.equal(nonceLock.nextNonce, '0', `nonce should be 0 returned ${nonceLock.nextNonce}`)
await nonceLock.releaseLock()
})
})
@@ -55,7 +55,7 @@ describe('Nonce Tracker', function () {
txParams: { nonce: '0x01' },
}, { count: 5 })
- nonceTracker = generateNonceTrackerWith(pendingTxs, confirmedTxs)
+ nonceTracker = generateNonceTrackerWith(pendingTxs, confirmedTxs, '0x0')
})
it('should return nonce after those', async function () {
@@ -69,14 +69,14 @@ describe('Nonce Tracker', function () {
describe('when local confirmed count is higher than network nonce', function () {
beforeEach(function () {
const txGen = new MockTxGen()
- confirmedTxs = txGen.generate({ status: 'confirmed' }, { count: 2 })
- nonceTracker = generateNonceTrackerWith([], confirmedTxs)
+ 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, '2', `nonce should be 2 got ${nonceLock.nextNonce}`)
+ assert.equal(nonceLock.nextNonce, '3', `nonce should be 3 got ${nonceLock.nextNonce}`)
await nonceLock.releaseLock()
})
})
@@ -125,6 +125,43 @@ describe('Nonce Tracker', function () {
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()
+ const confirmedTxs = txGen.generate({ status: 'confirmed' }, { count: 10 })
+ const 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()
+ })
+ })
})
})
generated by cgit v1.2.3 (git 2.25.1) at 2024-12-29 22:28:50 +0800