From 7854321faeeb8184b3171e3b214f57d6e61c0814 Mon Sep 17 00:00:00 2001 From: vicnaum Date: Wed, 6 Dec 2017 22:49:24 +0100 Subject: Fix for #2644 - Specified Nonce isn't used Added nonceSpecified. And a check if nonce was specified - then we should use the specified nonce instead of generating a new one. --- app/scripts/controllers/transactions.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'app/scripts/controllers/transactions.js') diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index ce709bd28..060484b87 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -177,6 +177,7 @@ module.exports = class TransactionController extends EventEmitter { const txParams = txMeta.txParams // ensure value txMeta.gasPriceSpecified = Boolean(txParams.gasPrice) + txMeta.nonceSpecified = Boolean(txParams.nonce) const gasPrice = txParams.gasPrice || await this.query.gasPrice() txParams.gasPrice = ethUtil.addHexPrefix(gasPrice.toString(16)) txParams.value = txParams.value || '0x0' @@ -200,7 +201,11 @@ module.exports = class TransactionController extends EventEmitter { // wait for a nonce nonceLock = await this.nonceTracker.getNonceLock(fromAddress) // add nonce to txParams - txMeta.txParams.nonce = ethUtil.addHexPrefix(nonceLock.nextNonce.toString(16)) + if (txMeta.nonceSpecified) { + txMeta.txParams.nonce = ethUtil.addHexPrefix(txMeta.txParams.nonce.toString(16)) + } else { + txMeta.txParams.nonce = ethUtil.addHexPrefix(nonceLock.nextNonce.toString(16)) + } // add nonce debugging information to txMeta txMeta.nonceDetails = nonceLock.nonceDetails this.txStateManager.updateTx(txMeta, 'transactions#approveTransaction') -- cgit v1.2.3 From 553d713636d317b74d819d62b5b931bdf2d112d8 Mon Sep 17 00:00:00 2001 From: vicnaum Date: Thu, 7 Dec 2017 15:30:05 +0100 Subject: A more expressive way replaced ifs with ? : --- app/scripts/controllers/transactions.js | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) (limited to 'app/scripts/controllers/transactions.js') diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index 060484b87..6bdbd2cfc 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -201,11 +201,8 @@ module.exports = class TransactionController extends EventEmitter { // wait for a nonce nonceLock = await this.nonceTracker.getNonceLock(fromAddress) // add nonce to txParams - if (txMeta.nonceSpecified) { - txMeta.txParams.nonce = ethUtil.addHexPrefix(txMeta.txParams.nonce.toString(16)) - } else { - txMeta.txParams.nonce = ethUtil.addHexPrefix(nonceLock.nextNonce.toString(16)) - } + const nonce = txMeta.nonceSpecified ? txMeta.txParams.nonce : nonceLock.nextNonce + txMeta.txParams.nonce = ethUtil.addHexPrefix(nonce.toString(16)) // add nonce debugging information to txMeta txMeta.nonceDetails = nonceLock.nonceDetails this.txStateManager.updateTx(txMeta, 'transactions#approveTransaction') -- cgit v1.2.3 From 950ec9596c931055c3e0f2212f2733c9ca07739d Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Thu, 7 Dec 2017 16:13:38 -0500 Subject: Do not allow nonces larger than the next valid nonce To avoid situations where a user signs a transaction that will become surprisingly valid in the future. --- app/scripts/controllers/transactions.js | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'app/scripts/controllers/transactions.js') diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index bb408d445..6110b9c75 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -209,6 +209,10 @@ module.exports = class TransactionController extends EventEmitter { 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) + } txMeta.txParams.nonce = ethUtil.addHexPrefix(nonce.toString(16)) // add nonce debugging information to txMeta txMeta.nonceDetails = nonceLock.nonceDetails -- cgit v1.2.3 From a91200fd08b429c81d4096de17cdd9066a632ade Mon Sep 17 00:00:00 2001 From: kumavis Date: Thu, 7 Dec 2017 18:04:14 -0500 Subject: tx-controller - failed state is a finished state --- app/scripts/controllers/transactions.js | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) (limited to 'app/scripts/controllers/transactions.js') diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index 6110b9c75..f95b5e39a 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -138,18 +138,20 @@ module.exports = class TransactionController extends EventEmitter { async newUnapprovedTransaction (txParams) { log.debug(`MetaMaskController newUnapprovedTransaction ${JSON.stringify(txParams)}`) - const txMeta = await this.addUnapprovedTransaction(txParams) - this.emit('newUnapprovedTx', txMeta) + const initialTxMeta = await this.addUnapprovedTransaction(txParams) + this.emit('newUnapprovedTx', initialTxMeta) // listen for tx completion (success, fail) return new Promise((resolve, reject) => { - this.txStateManager.once(`${txMeta.id}:finished`, (completedTx) => { - switch (completedTx.status) { + this.txStateManager.once(`${initialTxMeta.id}:finished`, (finishedTxMeta) => { + switch (finishedTxMeta.status) { case 'submitted': - return resolve(completedTx.hash) + return resolve(finishedTxMeta.hash) case 'rejected': return reject(new Error('MetaMask Tx Signature: User denied transaction signature.')) + case 'failed': + return reject(new Error(finishedTxMeta.err.message)) default: - return reject(new Error(`MetaMask Tx Signature: Unknown problem: ${JSON.stringify(completedTx.txParams)}`)) + return reject(new Error(`MetaMask Tx Signature: Unknown problem: ${JSON.stringify(finishedTxMeta.txParams)}`)) } }) }) -- cgit v1.2.3