Merge pull request #1969 from MetaMask/i1966-IgnoreInvalidPendingNonces
I1966 ignore invalid pending nonces
This commit is contained in:
commit
4de977e63e
|
@ -2,6 +2,8 @@
|
|||
|
||||
## Current Master
|
||||
|
||||
- Fix nonce calculation bug that would sometimes generate very wrong nonces.
|
||||
|
||||
## 3.9.10 2017-8-23
|
||||
|
||||
- Improve nonce calculation, to prevent bug where people are unable to send transactions reliably.
|
||||
|
|
|
@ -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)
|
||||
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 () {
|
||||
|
|
|
@ -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()
|
||||
})
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
|
|
Loading…
Reference in New Issue