From 60bc28bf3c75135bd751852e32ff98b7b6181249 Mon Sep 17 00:00:00 2001 From: frankiebee Date: Tue, 22 May 2018 16:37:30 -0700 Subject: [PATCH 1/3] test pending-tx-tracker - update tests to reflect new block tracker behavior and remove tx:confirmed event tests --- test/unit/pending-tx-test.js | 53 ++++++++++++++---------------------- 1 file changed, 21 insertions(+), 32 deletions(-) diff --git a/test/unit/pending-tx-test.js b/test/unit/pending-tx-test.js index 001b86dd1..ffd9a7154 100644 --- a/test/unit/pending-tx-test.js +++ b/test/unit/pending-tx-test.js @@ -13,7 +13,7 @@ const otherNetworkId = 36 const privKey = new Buffer('8718b9618a37d1fc78c436511fc6df3c8258d3250635bba617f33003270ec03e', 'hex') -describe('PendingTransactionTracker', function () { +describe.only('PendingTransactionTracker', function () { let pendingTxTracker, txMeta, txMetaNoHash, txMetaNoRawTx, providerResultStub, provider, txMeta3, txList, knownErrors this.timeout(10000) @@ -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 () { @@ -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() }) }) @@ -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 () { @@ -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) => { @@ -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 =[ @@ -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) => { @@ -270,7 +259,7 @@ describe('PendingTransactionTracker', function () { .then((txCompletedList) => done()) .catch(done) - pendingTxTracker.resubmitPendingTxs(blockStub) + pendingTxTracker.resubmitPendingTxs(blockNuberStub) }) }) describe('#_resubmitTx', function () { From 10aecf49227098fb7fdbd7db193a9dcc6fecf5af Mon Sep 17 00:00:00 2001 From: frankiebee Date: Tue, 22 May 2018 16:40:01 -0700 Subject: [PATCH 2/3] remove dependance on the even tx:confirmed --- app/scripts/controllers/transactions/index.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/app/scripts/controllers/transactions/index.js b/app/scripts/controllers/transactions/index.js index cb3d28f1d..313b20675 100644 --- a/app/scripts/controllers/transactions/index.js +++ b/app/scripts/controllers/transactions/index.js @@ -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() @@ -316,6 +317,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 @@ -396,8 +402,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) { From c4b09da34e9950ea485dfecb810726e49b683dcd Mon Sep 17 00:00:00 2001 From: frankiebee Date: Tue, 22 May 2018 16:41:00 -0700 Subject: [PATCH 3/3] transactions - update pending-tx-tracker to use the new block tracker --- .../transactions/pending-tx-tracker.js | 25 +++++++++++++------ test/unit/pending-tx-test.js | 2 +- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/app/scripts/controllers/transactions/pending-tx-tracker.js b/app/scripts/controllers/transactions/pending-tx-tracker.js index 6e2fcb40b..bd26a72d9 100644 --- a/app/scripts/controllers/transactions/pending-tx-tracker.js +++ b/app/scripts/controllers/transactions/pending-tx-tracker.js @@ -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() } @@ -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) => { @@ -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) + } }) }) } @@ -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() } @@ -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 @@ -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 = { @@ -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) diff --git a/test/unit/pending-tx-test.js b/test/unit/pending-tx-test.js index ffd9a7154..07029e0f7 100644 --- a/test/unit/pending-tx-test.js +++ b/test/unit/pending-tx-test.js @@ -13,7 +13,7 @@ const otherNetworkId = 36 const privKey = new Buffer('8718b9618a37d1fc78c436511fc6df3c8258d3250635bba617f33003270ec03e', 'hex') -describe.only('PendingTransactionTracker', function () { +describe('PendingTransactionTracker', function () { let pendingTxTracker, txMeta, txMetaNoHash, txMetaNoRawTx, providerResultStub, provider, txMeta3, txList, knownErrors this.timeout(10000)