From 62febac87659ddf78a34dd0dac1ee8a38d8c8e77 Mon Sep 17 00:00:00 2001 From: frankiebee Date: Tue, 27 Feb 2018 15:14:18 -0800 Subject: refactor retrytx with higher gas price: - create a new tx instead of overwriting the tx hash - add a new state 'dropped' to the txStateManager - mark duplicate txs as dropped when one gets confirmed in a block --- app/scripts/controllers/transactions.js | 43 +++++++++++++++++++++------------ 1 file changed, 27 insertions(+), 16 deletions(-) (limited to 'app/scripts/controllers/transactions.js') diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index ef5578d5a..aad3f9952 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -6,7 +6,6 @@ const EthQuery = require('ethjs-query') const TransactionStateManger = require('../lib/tx-state-manager') const TxGasUtil = require('../lib/tx-gas-utils') const PendingTransactionTracker = require('../lib/pending-tx-tracker') -const createId = require('../lib/random-id') const NonceTracker = require('../lib/nonce-tracker') /* @@ -92,8 +91,22 @@ module.exports = 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) + // get the confirmed transactions nonce and from address + const txMeta = this.txStateManager.getTx(txId) + const { nonce, from } = txMeta.txParams + const sameNonceTxs = this.txStateManager.getFilteredTxList({nonce, from}) + if (!sameNonceTxs.length) return + // mark all same nonce transactions as dropped and give i a replacedBy hash + sameNonceTxs.forEach((otherTxMeta) => { + if (otherTxMeta === txId) return + otherTxMeta.replacedBy = txMeta.hash + this.txStateManager.updateTx(txMeta, 'transactions/pending-tx-tracker#event: tx:confirmed reference to confirmed txHash with same nonce') + this.txStateManager.setTxStatusDropped(otherTxMeta.id) + }) + }) this.pendingTxTracker.on('tx:failed', this.txStateManager.setTxStatusFailed.bind(this.txStateManager)) - this.pendingTxTracker.on('tx:confirmed', this.txStateManager.setTxStatusConfirmed.bind(this.txStateManager)) this.pendingTxTracker.on('tx:block-update', (txMeta, latestBlockNumber) => { if (!txMeta.firstRetryBlockNumber) { txMeta.firstRetryBlockNumber = latestBlockNumber @@ -186,14 +199,7 @@ module.exports = class TransactionController extends EventEmitter { // validate await this.txGasUtil.validateTxParams(txParams) // construct txMeta - const txMeta = { - id: createId(), - time: (new Date()).getTime(), - status: 'unapproved', - metamaskNetworkId: this.getNetwork(), - txParams: txParams, - loadingDefaults: true, - } + const txMeta = this.txStateManager.generateTxMeta({txParams}) this.addTx(txMeta) this.emit('newUnapprovedTx', txMeta) // add default tx params @@ -215,7 +221,6 @@ module.exports = class TransactionController extends EventEmitter { const txParams = txMeta.txParams // ensure value txMeta.gasPriceSpecified = Boolean(txParams.gasPrice) - txMeta.nonceSpecified = Boolean(txParams.nonce) let gasPrice = txParams.gasPrice if (!gasPrice) { gasPrice = this.getGasPrice ? this.getGasPrice() : await this.query.gasPrice() @@ -226,11 +231,17 @@ module.exports = class TransactionController extends EventEmitter { return await this.txGasUtil.analyzeGasUsage(txMeta) } - async retryTransaction (txId) { - this.txStateManager.setTxStatusUnapproved(txId) - const txMeta = this.txStateManager.getTx(txId) - txMeta.lastGasPrice = txMeta.txParams.gasPrice - this.txStateManager.updateTx(txMeta, 'retryTransaction: manual retry') + async retryTransaction (originalTxId) { + const originalTxMeta = this.txStateManager.getTx(originalTxId) + const lastGasPrice = originalTxMeta.txParams.gasPrice + const txMeta = this.txStateManager.generateTxMeta({ + txParams: originalTxMeta.txParams, + lastGasPrice, + loadingDefaults: false, + nonceSpecified: true, + }) + this.addTx(txMeta) + this.emit('newUnapprovedTx', txMeta) } async updateTransaction (txMeta) { -- cgit From 4a3288fec9ae7bd717b539d9bfcb8d5ba5b1ef8c Mon Sep 17 00:00:00 2001 From: frankiebee Date: Wed, 7 Mar 2018 22:01:14 -0800 Subject: transactions - make _markNonceDuplicatesDropped --- app/scripts/controllers/transactions.js | 41 ++++++++++++++++----------------- 1 file changed, 20 insertions(+), 21 deletions(-) (limited to 'app/scripts/controllers/transactions.js') diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index aad3f9952..e903c73e8 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -91,21 +91,7 @@ module.exports = 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) - // get the confirmed transactions nonce and from address - const txMeta = this.txStateManager.getTx(txId) - const { nonce, from } = txMeta.txParams - const sameNonceTxs = this.txStateManager.getFilteredTxList({nonce, from}) - if (!sameNonceTxs.length) return - // mark all same nonce transactions as dropped and give i a replacedBy hash - sameNonceTxs.forEach((otherTxMeta) => { - if (otherTxMeta === txId) return - otherTxMeta.replacedBy = txMeta.hash - this.txStateManager.updateTx(txMeta, 'transactions/pending-tx-tracker#event: tx:confirmed reference to confirmed txHash with same nonce') - this.txStateManager.setTxStatusDropped(otherTxMeta.id) - }) - }) + 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) { @@ -238,7 +224,6 @@ module.exports = class TransactionController extends EventEmitter { txParams: originalTxMeta.txParams, lastGasPrice, loadingDefaults: false, - nonceSpecified: true, }) this.addTx(txMeta) this.emit('newUnapprovedTx', txMeta) @@ -264,11 +249,9 @@ module.exports = class TransactionController extends EventEmitter { // wait for a nonce nonceLock = await this.nonceTracker.getNonceLock(fromAddress) // add nonce to txParams - const nonce = txMeta.nonceSpecified ? txMeta.txParams.nonce : nonceLock.nextNonce - if (nonce > nonceLock.nextNonce) { - const message = `Specified nonce may not be larger than account's next valid nonce.` - throw new Error(message) - } + // if txMeta has lastGasPrice then it is a retry at same nonce with higher + // gas price transaction and their for the nonce should not be calculated + const nonce = txMeta.lastGasPrice ? txMeta.txParams.nonce : nonceLock.nextNonce txMeta.txParams.nonce = ethUtil.addHexPrefix(nonce.toString(16)) // add nonce debugging information to txMeta txMeta.nonceDetails = nonceLock.nonceDetails @@ -325,6 +308,22 @@ module.exports = class TransactionController extends EventEmitter { // PRIVATE METHODS // + _markNonceDuplicatesDropped (txId) { + this.txStateManager.setTxStatusConfirmed(txId) + // get the confirmed transactions nonce and from address + const txMeta = this.txStateManager.getTx(txId) + const { nonce, from } = txMeta.txParams + const sameNonceTxs = this.txStateManager.getFilteredTxList({nonce, from}) + if (!sameNonceTxs.length) return + // mark all same nonce transactions as dropped and give i a replacedBy hash + sameNonceTxs.forEach((otherTxMeta) => { + if (otherTxMeta === txId) return + otherTxMeta.replacedBy = txMeta.hash + this.txStateManager.updateTx(txMeta, 'transactions/pending-tx-tracker#event: tx:confirmed reference to confirmed txHash with same nonce') + this.txStateManager.setTxStatusDropped(otherTxMeta.id) + }) + } + _updateMemstore () { const unapprovedTxs = this.txStateManager.getUnapprovedTxList() const selectedAddressTxList = this.txStateManager.getFilteredTxList({ -- cgit From 5572345b781ce68178dd4e72d4e56b2dec26a454 Mon Sep 17 00:00:00 2001 From: frankiebee Date: Thu, 8 Mar 2018 10:36:31 -0800 Subject: fix marking of confirmed transaction as dropped --- app/scripts/controllers/transactions.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'app/scripts/controllers/transactions.js') diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index e903c73e8..3dbd424ca 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -317,7 +317,7 @@ module.exports = class TransactionController extends EventEmitter { if (!sameNonceTxs.length) return // mark all same nonce transactions as dropped and give i a replacedBy hash sameNonceTxs.forEach((otherTxMeta) => { - if (otherTxMeta === txId) return + if (otherTxMeta.id === txId) return otherTxMeta.replacedBy = txMeta.hash this.txStateManager.updateTx(txMeta, 'transactions/pending-tx-tracker#event: tx:confirmed reference to confirmed txHash with same nonce') this.txStateManager.setTxStatusDropped(otherTxMeta.id) -- cgit From 9d7640996aa9b63fb6c9271d7e0698e7acdc559e Mon Sep 17 00:00:00 2001 From: frankiebee Date: Tue, 13 Mar 2018 14:42:26 -0700 Subject: transactions - return the txMeta in retryTransaction --- app/scripts/controllers/transactions.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'app/scripts/controllers/transactions.js') diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index a3f731b6e..ab07dde62 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -3,7 +3,7 @@ const ObservableStore = require('obs-store') const ethUtil = require('ethereumjs-util') const Transaction = require('ethereumjs-tx') const EthQuery = require('ethjs-query') -const TransactionStateManager = require('../lib/tx-state-manager') +const TransactionStateManger = require('../lib/tx-state-manager') const TxGasUtil = require('../lib/tx-gas-utils') const PendingTransactionTracker = require('../lib/pending-tx-tracker') const NonceTracker = require('../lib/nonce-tracker') @@ -37,7 +37,7 @@ module.exports = class TransactionController extends EventEmitter { this.query = new EthQuery(this.provider) this.txGasUtil = new TxGasUtil(this.provider) - this.txStateManager = new TransactionStateManager({ + this.txStateManager = new TransactionStateManger({ initState: opts.initState, txHistoryLimit: opts.txHistoryLimit, getNetwork: this.getNetwork.bind(this), @@ -227,6 +227,7 @@ module.exports = class TransactionController extends EventEmitter { }) this.addTx(txMeta) this.emit('newUnapprovedTx', txMeta) + return txMeta } async updateTransaction (txMeta) { -- cgit From 106ce091a9ef9fbd37b90c49bc76f12bd796ebb5 Mon Sep 17 00:00:00 2001 From: Dan Date: Wed, 14 Mar 2018 11:45:04 -0230 Subject: Fix TransactionStateManager spelling. --- app/scripts/controllers/transactions.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'app/scripts/controllers/transactions.js') diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index ab07dde62..3e3909361 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -3,7 +3,7 @@ const ObservableStore = require('obs-store') const ethUtil = require('ethereumjs-util') const Transaction = require('ethereumjs-tx') const EthQuery = require('ethjs-query') -const TransactionStateManger = require('../lib/tx-state-manager') +const TransactionStateManager = require('../lib/tx-state-manager') const TxGasUtil = require('../lib/tx-gas-utils') const PendingTransactionTracker = require('../lib/pending-tx-tracker') const NonceTracker = require('../lib/nonce-tracker') @@ -37,7 +37,7 @@ module.exports = class TransactionController extends EventEmitter { this.query = new EthQuery(this.provider) this.txGasUtil = new TxGasUtil(this.provider) - this.txStateManager = new TransactionStateManger({ + this.txStateManager = new TransactionStateManager({ initState: opts.initState, txHistoryLimit: opts.txHistoryLimit, getNetwork: this.getNetwork.bind(this), -- cgit