From d97c6533b87b0a9dd6937c1ca57ec05129ac619b Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Tue, 11 Jul 2017 11:59:56 -0700 Subject: [PATCH 1/2] Remove local nonce error setting. --- CHANGELOG.md | 2 ++ app/scripts/controllers/transactions.js | 8 -------- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 696d68345..f53bdead5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## Current Master +- No longer validate nonce client-side in retry loop. + ## 3.8.4 2017-7-7 - Improve transaction resubmit logic to fail more eagerly when a user would expect it to. diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index 18bb245de..02487c385 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -458,14 +458,6 @@ module.exports = class TransactionController extends EventEmitter { return log.error(message) } - // if the nonce of the transaction is lower then the accounts nonce, fail. - if (txNonce < nonce) { - const message = 'Invalid nonce.' - this.setTxStatusFailed(txMeta.id, { message }) - cb() - return log.error(message) - } - // Only auto-submit already-signed txs: if (!('rawTx' in txMeta)) return cb() From c7b9e3fb1878cebbab26d5343cc18084a601c6bb Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Tue, 11 Jul 2017 12:18:07 -0700 Subject: [PATCH 2/2] Improve insufficient balance checking in retry loop --- CHANGELOG.md | 1 + app/scripts/controllers/transactions.js | 5 +--- app/scripts/lib/tx-utils.js | 9 ++++++ test/unit/tx-utils-test.js | 38 +++++++++++++++++++++++++ 4 files changed, 49 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f53bdead5..395454b41 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## Current Master - No longer validate nonce client-side in retry loop. +- Fix bug where insufficient balance error was sometimes shown on successful transactions. ## 3.8.4 2017-7-7 diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index 02487c385..ca379a7ff 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -445,13 +445,10 @@ module.exports = class TransactionController extends EventEmitter { _resubmitTx (txMeta, cb) { const address = txMeta.txParams.from const balance = this.ethStore.getState().accounts[address].balance - const nonce = Number.parseInt(this.ethStore.getState().accounts[address].nonce) - const txNonce = Number.parseInt(txMeta.txParams.nonce) - const gtBalance = Number.parseInt(txMeta.txParams.value) > Number.parseInt(balance) if (!('retryCount' in txMeta)) txMeta.retryCount = 0 // if the value of the transaction is greater then the balance, fail. - if (gtBalance) { + if (!this.txProviderUtils.sufficientBalance(txMeta.txParams, balance)) { const message = 'Insufficient balance.' this.setTxStatusFailed(txMeta.id, { message }) cb() diff --git a/app/scripts/lib/tx-utils.js b/app/scripts/lib/tx-utils.js index 149d93102..4e780fcc0 100644 --- a/app/scripts/lib/tx-utils.js +++ b/app/scripts/lib/tx-utils.js @@ -118,6 +118,15 @@ module.exports = class txProviderUtils { } } + sufficientBalance (tx, hexBalance) { + const balance = hexToBn(hexBalance) + const value = hexToBn(tx.value) + const gasLimit = hexToBn(tx.gas) + const gasPrice = hexToBn(tx.gasPrice) + + const maxCost = value.add(gasLimit.mul(gasPrice)) + return balance.gte(maxCost) + } } diff --git a/test/unit/tx-utils-test.js b/test/unit/tx-utils-test.js index 7ace1f587..a43bcfb35 100644 --- a/test/unit/tx-utils-test.js +++ b/test/unit/tx-utils-test.js @@ -16,6 +16,44 @@ describe('txUtils', function () { })) }) + describe('#sufficientBalance', function () { + it('returns true if max tx cost is equal to balance.', function () { + const tx = { + 'value': '0x1', + 'gas': '0x2', + 'gasPrice': '0x3', + } + const balance = '0x8' + + const result = txUtils.sufficientBalance(tx, balance) + assert.ok(result, 'sufficient balance found.') + }) + + it('returns true if max tx cost is less than balance.', function () { + const tx = { + 'value': '0x1', + 'gas': '0x2', + 'gasPrice': '0x3', + } + const balance = '0x9' + + const result = txUtils.sufficientBalance(tx, balance) + assert.ok(result, 'sufficient balance found.') + }) + + it('returns false if max tx cost is more than balance.', function () { + const tx = { + 'value': '0x1', + 'gas': '0x2', + 'gasPrice': '0x3', + } + const balance = '0x6' + + const result = txUtils.sufficientBalance(tx, balance) + assert.ok(!result, 'insufficient balance found.') + }) + }) + describe('chain Id', function () { it('prepares a transaction with the provided chainId', function () { const txParams = {