Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Transactions use new block tracker #4347

Merged
merged 4 commits into from
May 23, 2018
Merged

Transactions use new block tracker #4347

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
60bc28b
test pending-tx-tracker - update tests to reflect new block tracker b…
frankiebee May 22, 2018
10aecf4
remove dependance on the even tx:confirmed
frankiebee May 22, 2018
c4b09da
transactions - update pending-tx-tracker to use the new block tracker
frankiebee May 22, 2018
61caee9
Merge branch 'network-remove-provider-engine' of https://github.com/M…
frankiebee May 22, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions app/scripts/controllers/transactions/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ class TransactionController extends EventEmitter {
initState: opts.initState,
txHistoryLimit: opts.txHistoryLimit,
getNetwork: this.getNetwork.bind(this),
confirmTransaction: this.confirmTransaction.bind(this),
})
this._onBootCleanUp()

Expand Down Expand Up @@ -301,6 +302,11 @@ class TransactionController extends EventEmitter {
this.txStateManager.setTxStatusSubmitted(txId)
}

confirmTransaction (txId) {
this.txStateManager.setTxStatusConfirmed(txId)
this._markNonceDuplicatesDropped(txId)
}

/**
Convenience method for the ui thats sets the transaction to rejected
@param txId {number} - the tx's Id
Expand Down Expand Up @@ -381,8 +387,6 @@ class TransactionController extends EventEmitter {
this.pendingTxTracker.on('tx:warning', (txMeta) => {
this.txStateManager.updateTx(txMeta, 'transactions/pending-tx-tracker#event: tx:warning')
})
this.pendingTxTracker.on('tx:confirmed', (txId) => this.txStateManager.setTxStatusConfirmed(txId))
this.pendingTxTracker.on('tx:confirmed', (txId) => this._markNonceDuplicatesDropped(txId))
this.pendingTxTracker.on('tx:failed', this.txStateManager.setTxStatusFailed.bind(this.txStateManager))
this.pendingTxTracker.on('tx:block-update', (txMeta, latestBlockNumber) => {
if (!txMeta.firstRetryBlockNumber) {
Expand Down
25 changes: 18 additions & 7 deletions app/scripts/controllers/transactions/pending-tx-tracker.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ class PendingTransactionTracker extends EventEmitter {
this.getPendingTransactions = config.getPendingTransactions
this.getCompletedTransactions = config.getCompletedTransactions
this.publishTransaction = config.publishTransaction
this.confirmTransaction = config.confirmTransaction
this._checkPendingTxs()
}

Expand All @@ -37,7 +38,8 @@ class PendingTransactionTracker extends EventEmitter {
@emits tx:confirmed
@emits tx:failed
*/
checkForTxInBlock (block) {
async checkForTxInBlock (blockNumber) {
const block = await this._getBlock(blockNumber)
const signedTxList = this.getPendingTransactions()
if (!signedTxList.length) return
signedTxList.forEach((txMeta) => {
Expand All @@ -51,9 +53,12 @@ class PendingTransactionTracker extends EventEmitter {
return
}

if (!block.transactions.length) return

block.transactions.forEach((tx) => {
if (tx.hash === txHash) this.emit('tx:confirmed', txId)
block.transactions.forEach((hash) => {
if (hash === txHash) {
this.confirmTransaction(txId)
}
})
})
}
Expand All @@ -70,7 +75,7 @@ class PendingTransactionTracker extends EventEmitter {
return
}
// if we synced by more than one block, check for missed pending transactions
const diff = Number.parseInt(newBlock.number, 16) - Number.parseInt(oldBlock.number, 16)
const diff = Number.parseInt(newBlock, 16) - Number.parseInt(oldBlock, 16)
if (diff > 1) this._checkPendingTxs()
}

Expand All @@ -79,11 +84,11 @@ class PendingTransactionTracker extends EventEmitter {
@param block {object} - a block object
@emits tx:warning
*/
resubmitPendingTxs (block) {
resubmitPendingTxs (blockNumber) {
const pending = this.getPendingTransactions()
// only try resubmitting if their are transactions to resubmit
if (!pending.length) return
pending.forEach((txMeta) => this._resubmitTx(txMeta, block.number).catch((err) => {
pending.forEach((txMeta) => this._resubmitTx(txMeta, blockNumber).catch((err) => {
/*
Dont marked as failed if the error is a "known" transaction warning
"there is already a transaction with the same sender-nonce
Expand Down Expand Up @@ -179,7 +184,7 @@ class PendingTransactionTracker extends EventEmitter {
txParams = await this.query.getTransactionByHash(txHash)
if (!txParams) return
if (txParams.blockNumber) {
this.emit('tx:confirmed', txId)
this.confirmTransaction(txId)
}
} catch (err) {
txMeta.warning = {
Expand All @@ -206,11 +211,17 @@ class PendingTransactionTracker extends EventEmitter {
nonceGlobalLock.releaseLock()
}

async _getBlock (blockNumber) {
return await this.query.getBlockByNumber(blockNumber, false)
}

/**
checks to see if a confirmed txMeta has the same nonce
@param txMeta {Object} - txMeta object
@returns {boolean}
*/


async _checkIfNonceIsTaken (txMeta) {
const address = txMeta.txParams.from
const completed = this.getCompletedTransactions(address)
Expand Down
51 changes: 20 additions & 31 deletions test/unit/app/controllers/transactions/pending-tx-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,10 @@ describe('PendingTransactionTracker', function () {
getPendingTransactions: () => {return []},
getCompletedTransactions: () => {return []},
publishTransaction: () => {},
confirmTransaction: () => {},
})

pendingTxTracker._getBlock = (blockNumber) => { return {number: blockNumber, transactions: []} }
})

describe('_checkPendingTx state management', function () {
Expand Down Expand Up @@ -120,40 +123,36 @@ describe('PendingTransactionTracker', function () {
})
pendingTxTracker.checkForTxInBlock(block)
})
it('should emit \'txConfirmed\' if the tx is in the block', function (done) {
const block = { transactions: [txMeta]}
pendingTxTracker.getPendingTransactions = () => [txMeta]
pendingTxTracker.once('tx:confirmed', (txId) => {
assert(txId, txMeta.id, 'should pass txId')
done()
})
pendingTxTracker.once('tx:failed', (_, err) => { done(err) })
pendingTxTracker.checkForTxInBlock(block)
})
})
describe('#queryPendingTxs', function () {
it('should call #_checkPendingTxs if their is no oldBlock', function (done) {
let newBlock, oldBlock
newBlock = { number: '0x01' }
pendingTxTracker._checkPendingTxs = done
newBlock = '0x01'
const originalFunction = pendingTxTracker._checkPendingTxs
pendingTxTracker._checkPendingTxs = () => { done() }
pendingTxTracker.queryPendingTxs({ oldBlock, newBlock })
pendingTxTracker._checkPendingTxs = originalFunction
})
it('should call #_checkPendingTxs if oldBlock and the newBlock have a diff of greater then 1', function (done) {
let newBlock, oldBlock
oldBlock = { number: '0x01' }
newBlock = { number: '0x03' }
pendingTxTracker._checkPendingTxs = done
oldBlock = '0x01'
newBlock = '0x03'
const originalFunction = pendingTxTracker._checkPendingTxs
pendingTxTracker._checkPendingTxs = () => { done() }
pendingTxTracker.queryPendingTxs({ oldBlock, newBlock })
pendingTxTracker._checkPendingTxs = originalFunction
})
it('should not call #_checkPendingTxs if oldBlock and the newBlock have a diff of 1 or less', function (done) {
let newBlock, oldBlock
oldBlock = { number: '0x1' }
newBlock = { number: '0x2' }
oldBlock = '0x1'
newBlock = '0x2'
const originalFunction = pendingTxTracker._checkPendingTxs
pendingTxTracker._checkPendingTxs = () => {
const err = new Error('should not call #_checkPendingTxs if oldBlock and the newBlock have a diff of 1 or less')
done(err)
}
pendingTxTracker.queryPendingTxs({ oldBlock, newBlock })
pendingTxTracker._checkPendingTxs = originalFunction
done()
})
})
Expand All @@ -171,16 +170,6 @@ describe('PendingTransactionTracker', function () {
providerResultStub.eth_getTransactionByHash = null
pendingTxTracker._checkPendingTx(txMeta)
})

it('should emit \'txConfirmed\'', function (done) {
providerResultStub.eth_getTransactionByHash = {blockNumber: '0x01'}
pendingTxTracker.once('tx:confirmed', (txId) => {
assert(txId, txMeta.id, 'should pass txId')
done()
})
pendingTxTracker.once('tx:failed', (_, err) => { done(err) })
pendingTxTracker._checkPendingTx(txMeta)
})
})

describe('#_checkPendingTxs', function () {
Expand All @@ -207,7 +196,7 @@ describe('PendingTransactionTracker', function () {
})

describe('#resubmitPendingTxs', function () {
const blockStub = { number: '0x0' };
const blockNuberStub = '0x0'
beforeEach(function () {
const txMeta2 = txMeta3 = txMeta
txList = [txMeta, txMeta2, txMeta3].map((tx) => {
Expand All @@ -225,7 +214,7 @@ describe('PendingTransactionTracker', function () {
Promise.all(txList.map((tx) => tx.processed))
.then((txCompletedList) => done())
.catch(done)
pendingTxTracker.resubmitPendingTxs(blockStub)
pendingTxTracker.resubmitPendingTxs(blockNuberStub)
})
it('should not emit \'tx:failed\' if the txMeta throws a known txError', function (done) {
knownErrors =[
Expand All @@ -252,7 +241,7 @@ describe('PendingTransactionTracker', function () {
.then((txCompletedList) => done())
.catch(done)

pendingTxTracker.resubmitPendingTxs(blockStub)
pendingTxTracker.resubmitPendingTxs(blockNuberStub)
})
it('should emit \'tx:warning\' if it encountered a real error', function (done) {
pendingTxTracker.once('tx:warning', (txMeta, err) => {
Expand All @@ -270,7 +259,7 @@ describe('PendingTransactionTracker', function () {
.then((txCompletedList) => done())
.catch(done)

pendingTxTracker.resubmitPendingTxs(blockStub)
pendingTxTracker.resubmitPendingTxs(blockNuberStub)
})
})
describe('#_resubmitTx', function () {
Expand Down