From f47e81e4937871a7a690e357443a728b9049b8f0 Mon Sep 17 00:00:00 2001 From: frankiebee Date: Tue, 19 Dec 2017 14:28:02 -0800 Subject: [PATCH 01/18] transactions - throw error if dapp provides txParams whos value has a decimal --- app/scripts/lib/tx-gas-utils.js | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/app/scripts/lib/tx-gas-utils.js b/app/scripts/lib/tx-gas-utils.js index 56bee19f7..c00323a64 100644 --- a/app/scripts/lib/tx-gas-utils.js +++ b/app/scripts/lib/tx-gas-utils.js @@ -81,8 +81,15 @@ module.exports = class txProvideUtil { } async validateTxParams (txParams) { - if (('value' in txParams) && txParams.value.indexOf('-') === 0) { - throw new Error(`Invalid transaction value of ${txParams.value} not a positive number.`) + if ('value' in txParams) { + const value = txParams.value.toString() + if (value.indexOf('-') === 0) { + throw new Error(`Invalid transaction value of ${txParams.value} not a positive number.`) + } + + if (value.indexOf('.') >= 0) { + throw new Error(`Invalid transaction value of ${txParams.value} number must be in wei`) + } } } } From 9f08ada1a6d9ba15ecec1be83089d89222bbe6c1 Mon Sep 17 00:00:00 2001 From: frankiebee Date: Wed, 20 Dec 2017 18:49:52 -0800 Subject: [PATCH 02/18] add to CHANGELOG.md --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3b7256466..846e6d81e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## Current Master +- Throw an error if a application trys to submit a tx who's value contains a decimal and infor that it should be in wei. + ## 3.13.3 2017-12-14 - Show tokens that are held that have no balance. From cc7e85c078c0bd3ac69f4f2c2de07da8f4fb4d23 Mon Sep 17 00:00:00 2001 From: Dan Finlay <542863+danfinlay@users.noreply.github.com> Date: Thu, 21 Dec 2017 11:36:09 -0800 Subject: [PATCH 03/18] Spelling --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 846e6d81e..391cbe66c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ## Current Master -- Throw an error if a application trys to submit a tx who's value contains a decimal and infor that it should be in wei. +- Throw an error if a application tries to submit a tx whose value is a decimal, and inform that it should be in wei. ## 3.13.3 2017-12-14 From 37789f2dba3e0a9bceb01d6fe1cbce9f93fe2494 Mon Sep 17 00:00:00 2001 From: "greenkeeper[bot]" Date: Sat, 23 Dec 2017 05:35:28 +0000 Subject: [PATCH 04/18] chore(package): update vinyl-source-stream to version 2.0.0 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 7184e3ea8..871ed204e 100644 --- a/package.json +++ b/package.json @@ -217,7 +217,7 @@ "testem": "^1.10.3", "uglifyify": "^4.0.2", "vinyl-buffer": "^1.0.0", - "vinyl-source-stream": "^1.1.0", + "vinyl-source-stream": "^2.0.0", "watchify": "^3.9.0" }, "engines": { From 5efb0044d8e334d6c4ec2b5d68e830932eb96ed7 Mon Sep 17 00:00:00 2001 From: frankiebee Date: Wed, 27 Dec 2017 16:50:15 -0800 Subject: [PATCH 05/18] transactions:pending - only check nonces of transactions who's from adress match the txMeta --- app/scripts/lib/pending-tx-tracker.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/scripts/lib/pending-tx-tracker.js b/app/scripts/lib/pending-tx-tracker.js index 7956a3329..e8869e6b8 100644 --- a/app/scripts/lib/pending-tx-tracker.js +++ b/app/scripts/lib/pending-tx-tracker.js @@ -178,7 +178,8 @@ module.exports = class PendingTransactionTracker extends EventEmitter { } async _checkIfNonceIsTaken (txMeta) { - const completed = this.getCompletedTransactions() + const address = txMeta.txParams.from + const completed = this.getCompletedTransactions(address) const sameNonce = completed.filter((otherMeta) => { return otherMeta.txParams.nonce === txMeta.txParams.nonce }) From 3d627e869bb7ba1dc0316ad179f9fff07e5cb83c Mon Sep 17 00:00:00 2001 From: Kevin Serrano Date: Wed, 27 Dec 2017 17:26:38 -0800 Subject: [PATCH 06/18] Add test for edge case. --- test/unit/util_test.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/unit/util_test.js b/test/unit/util_test.js index 3a8b6bdfd..1d6e38c5d 100644 --- a/test/unit/util_test.js +++ b/test/unit/util_test.js @@ -201,6 +201,12 @@ describe('util', function () { var output = util.normalizeEthStringToWei(input) assert.equal(output.toString(10), ethInWei) }) + + it('should account for overflow numbers gracefully by dropping extra precision.', function () { + var input = '1.11111111111111111111' + var output = util.normalizeEthStringToWei(input) + assert.equal(output.toString(10), '1111111111111111111') + }) }) describe('#normalizeNumberToWei', function () { From 414f89668eb554e82ab22d3b3080322057388266 Mon Sep 17 00:00:00 2001 From: Kevin Serrano Date: Wed, 27 Dec 2017 17:27:48 -0800 Subject: [PATCH 07/18] Fix some silly linting issues. --- app/scripts/lib/tx-gas-utils.js | 2 +- app/scripts/notice-controller.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/scripts/lib/tx-gas-utils.js b/app/scripts/lib/tx-gas-utils.js index 56bee19f7..247b34e47 100644 --- a/app/scripts/lib/tx-gas-utils.js +++ b/app/scripts/lib/tx-gas-utils.js @@ -26,7 +26,7 @@ module.exports = class txProvideUtil { err.message.includes('Transaction execution error.') || err.message.includes('gas required exceeds allowance or always failing transaction') ) - if ( simulationFailed ) { + if (simulationFailed) { txMeta.simulationFails = true return txMeta } diff --git a/app/scripts/notice-controller.js b/app/scripts/notice-controller.js index db2b8c4f4..14a63eae7 100644 --- a/app/scripts/notice-controller.js +++ b/app/scripts/notice-controller.js @@ -77,7 +77,7 @@ module.exports = class NoticeController extends EventEmitter { return uniqBy(oldNotices.concat(newNotices), 'id') } - _filterNotices(notices) { + _filterNotices (notices) { return notices.filter((newNotice) => { if ('version' in newNotice) { const satisfied = semver.satisfies(this.version, newNotice.version) From 3e562f1a1e594955f203b75a0be595e4bd6bca76 Mon Sep 17 00:00:00 2001 From: Kevin Serrano Date: Wed, 27 Dec 2017 17:31:38 -0800 Subject: [PATCH 08/18] Add backend fix for util in normalizeethstringtowei. --- ui/app/util.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ui/app/util.js b/ui/app/util.js index 3f8b4dcc3..293f4228c 100644 --- a/ui/app/util.js +++ b/ui/app/util.js @@ -193,6 +193,9 @@ function normalizeEthStringToWei (str) { while (decimal.length < 18) { decimal += '0' } + if (decimal.length > 18) { + decimal = decimal.slice(0, 18) + } const decimalBN = new ethUtil.BN(decimal, 10) eth = eth.add(decimalBN) } From adf4b89804af8b8f4e9a543a912f548715720d64 Mon Sep 17 00:00:00 2001 From: Kevin Serrano Date: Wed, 27 Dec 2017 17:33:28 -0800 Subject: [PATCH 09/18] Add additional test to ui utils to account for exact wei values. --- test/unit/util_test.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/unit/util_test.js b/test/unit/util_test.js index 1d6e38c5d..59048975a 100644 --- a/test/unit/util_test.js +++ b/test/unit/util_test.js @@ -207,6 +207,12 @@ describe('util', function () { var output = util.normalizeEthStringToWei(input) assert.equal(output.toString(10), '1111111111111111111') }) + + it('should not truncate very exact wei values that do not have extra precision.', function () { + var input = '1.100000000000000001' + var output = util.normalizeEthStringToWei(input) + assert.equal(output.toString(10), '1100000000000000001') + }) }) describe('#normalizeNumberToWei', function () { From 92eb16fc112eeac749c0ddfff163773ce3a35ef2 Mon Sep 17 00:00:00 2001 From: Kevin Serrano Date: Wed, 27 Dec 2017 17:36:56 -0800 Subject: [PATCH 10/18] Add frontend validation to check if send ether input is a valid number. --- ui/app/send.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/ui/app/send.js b/ui/app/send.js index e59c1130e..7b31eef7e 100644 --- a/ui/app/send.js +++ b/ui/app/send.js @@ -247,6 +247,12 @@ SendTransactionScreen.prototype.onSubmit = function () { const recipient = state.recipient || document.querySelector('input[name="address"]').value.replace(/^[.\s]+|[.\s]+$/g, '') const nickname = state.nickname || ' ' const input = document.querySelector('input[name="amount"]').value + + if (isNaN(input)) { + message = 'Invalid ether value.' + return this.props.dispatch(actions.displayWarning(message)) + } + const value = util.normalizeEthStringToWei(input) const txData = document.querySelector('input[name="txData"]').value const balance = this.props.balance From 8bd942d40629d8d68dd98b0bb8d10bf60d3e92c7 Mon Sep 17 00:00:00 2001 From: frankiebee Date: Wed, 27 Dec 2017 17:45:03 -0800 Subject: [PATCH 11/18] add tests for #_checkIfNonceIsTaken --- test/unit/pending-tx-test.js | 63 ++++++++++++++++++++++++++++++++++-- 1 file changed, 61 insertions(+), 2 deletions(-) diff --git a/test/unit/pending-tx-test.js b/test/unit/pending-tx-test.js index 393601a57..0b557f055 100644 --- a/test/unit/pending-tx-test.js +++ b/test/unit/pending-tx-test.js @@ -328,7 +328,7 @@ describe('PendingTransactionTracker', function () { it('should publish the transaction if the number of blocks since last retry exceeds the last set limit', function (done) { const enoughBalance = '0x100000' const mockLatestBlockNumber = '0x11' - + pendingTxTracker._resubmitTx(txMetaToTestExponentialBackoff, mockLatestBlockNumber) .then(() => done()) .catch((err) => { @@ -338,5 +338,64 @@ describe('PendingTransactionTracker', function () { assert.equal(pendingTxTracker.publishTransaction.callCount, 1, 'Should call publish transaction') }) - }) + }) + + describe('#_checkIfNonceIsTaken', function () { + beforeEach ( function () { + let confirmedTxList = [{ + id: 1, + hash: '0x0593ee121b92e10d63150ad08b4b8f9c7857d1bd160195ee648fb9a0f8d00eeb', + status: 'confirmed', + txParams: { + from: '0x1678a085c290ebd122dc42cba69373b5953b831d', + nonce: '0x1', + value: '0xfffff', + }, + rawTx: '0xf86c808504a817c800827b0d940c62bb85faa3311a998d3aba8098c1235c564966880de0b6b3a7640000802aa08ff665feb887a25d4099e40e11f0fef93ee9608f404bd3f853dd9e84ed3317a6a02ec9d3d1d6e176d4d2593dd760e74ccac753e6a0ea0d00cc9789d0d7ff1f471d', + }, { + id: 2, + hash: '0x0593ee121b92e10d63150ad08b4b8f9c7857d1bd160195ee648fb9a0f8d00eeb', + status: 'confirmed', + txParams: { + from: '0x1678a085c290ebd122dc42cba69373b5953b831d', + nonce: '0x2', + value: '0xfffff', + }, + rawTx: '0xf86c808504a817c800827b0d940c62bb85faa3311a998d3aba8098c1235c564966880de0b6b3a7640000802aa08ff665feb887a25d4099e40e11f0fef93ee9608f404bd3f853dd9e84ed3317a6a02ec9d3d1d6e176d4d2593dd760e74ccac753e6a0ea0d00cc9789d0d7ff1f471d', + }] + pendingTxTracker.getCompletedTransactions = (address) => { + if (!address) throw new Error('unless behavior has changed #_checkIfNonceIsTaken needs a filtered list of transactions to see if the nonce is taken') + return confirmedTxList + } + }) + + it('should return false', function (done) { + pendingTxTracker._checkIfNonceIsTaken({ + txParams: { + from: '0x1678a085c290ebd122dc42cba69373b5953b831d', + nonce: '0x3', + value: '0xfffff', + }, + }) + .then((taken) => { + assert.ok(!taken) + done() + }) + .catch(done) + }) + + it('should return true', function (done) { + pendingTxTracker._checkIfNonceIsTaken({ + txParams: { + from: '0x1678a085c290ebd122dc42cba69373b5953b831d', + nonce: '0x2', + value: '0xfffff', + }, + }).then((taken) => { + assert.ok(taken) + done() + }) + .catch(done) + }) + }) }) From e682fb05fcdeca5f50a2176934d25935e13538f6 Mon Sep 17 00:00:00 2001 From: Kevin Serrano Date: Wed, 27 Dec 2017 18:22:10 -0800 Subject: [PATCH 12/18] Add frontend validation to ensure that ether inputs are valid. --- ui/app/send.js | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/ui/app/send.js b/ui/app/send.js index 7b31eef7e..09c9e03d4 100644 --- a/ui/app/send.js +++ b/ui/app/send.js @@ -247,16 +247,26 @@ SendTransactionScreen.prototype.onSubmit = function () { const recipient = state.recipient || document.querySelector('input[name="address"]').value.replace(/^[.\s]+|[.\s]+$/g, '') const nickname = state.nickname || ' ' const input = document.querySelector('input[name="amount"]').value + const parts = input.split('') - if (isNaN(input)) { + let message + + if (isNaN(input) || input === '') { message = 'Invalid ether value.' return this.props.dispatch(actions.displayWarning(message)) } + if (parts[1]) { + var decimal = parts[1] + if (decimal.length > 18) { + message = 'Ether amount is too precise.' + return this.props.dispatch(actions.displayWarning(message)) + } + } + const value = util.normalizeEthStringToWei(input) const txData = document.querySelector('input[name="txData"]').value const balance = this.props.balance - let message if (value.gt(balance)) { message = 'Insufficient funds.' From c3d85c0a66676a15b598424e379610237243cb10 Mon Sep 17 00:00:00 2001 From: Kevin Serrano Date: Wed, 27 Dec 2017 18:23:05 -0800 Subject: [PATCH 13/18] Bump Changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3c1c1dcb4..dfcd0d1b9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ - Fix bug that prevented updating custom token details. - No longer mark long-pending transactions as failed, since we now have button to retry with higher gas. +- Fix rounding error when specifying an ether amount that has too much precision. ## 3.13.3 2017-12-14 From 0c54efdfc74079ff539606cbd0fc0a08fa922a0e Mon Sep 17 00:00:00 2001 From: frankiebee Date: Thu, 28 Dec 2017 15:48:22 -0800 Subject: [PATCH 14/18] tests - be more verbose in test messages --- test/unit/pending-tx-test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/unit/pending-tx-test.js b/test/unit/pending-tx-test.js index 0b557f055..bd47299cf 100644 --- a/test/unit/pending-tx-test.js +++ b/test/unit/pending-tx-test.js @@ -369,7 +369,7 @@ describe('PendingTransactionTracker', function () { } }) - it('should return false', function (done) { + it('should return false if nonce has not been taken', function (done) { pendingTxTracker._checkIfNonceIsTaken({ txParams: { from: '0x1678a085c290ebd122dc42cba69373b5953b831d', @@ -384,7 +384,7 @@ describe('PendingTransactionTracker', function () { .catch(done) }) - it('should return true', function (done) { + it('should return true if nonce has been taken', function (done) { pendingTxTracker._checkIfNonceIsTaken({ txParams: { from: '0x1678a085c290ebd122dc42cba69373b5953b831d', From 4a598fc7ac59ccf4c6cc0290956bb4985fafa8a5 Mon Sep 17 00:00:00 2001 From: Renier Oosthuizen Date: Tue, 2 Jan 2018 21:07:48 +0200 Subject: [PATCH 15/18] Fixes #2834 --- ui/app/app.js | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/ui/app/app.js b/ui/app/app.js index bd0ccb0a2..2a2438d97 100644 --- a/ui/app/app.js +++ b/ui/app/app.js @@ -460,11 +460,6 @@ App.prototype.renderPrimary = function () { }) } - if (props.seedWords) { - log.debug('rendering seed words') - return h(HDCreateVaultComplete, {key: 'HDCreateVaultComplete'}) - } - // show initialize screen if (!props.isInitialized || props.forgottenPassword) { // show current view @@ -499,6 +494,12 @@ App.prototype.renderPrimary = function () { } } + //Show seed words screen + if (props.seedWords) { + log.debug('rendering seed words') + return h(HDCreateVaultComplete, {key: 'HDCreateVaultComplete'}) + } + // show current view switch (props.currentView.name) { From 1da385f78b9e61e7a17c19164558779a37376868 Mon Sep 17 00:00:00 2001 From: oosthuizenr Date: Tue, 2 Jan 2018 21:29:26 +0200 Subject: [PATCH 16/18] fix comment --- ui/app/app.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/app/app.js b/ui/app/app.js index 2a2438d97..bc198b482 100644 --- a/ui/app/app.js +++ b/ui/app/app.js @@ -494,7 +494,7 @@ App.prototype.renderPrimary = function () { } } - //Show seed words screen + // show seed words screen if (props.seedWords) { log.debug('rendering seed words') return h(HDCreateVaultComplete, {key: 'HDCreateVaultComplete'}) From 3f6cef0b3f56edc8cc0b66403f23ae216de7bcfa Mon Sep 17 00:00:00 2001 From: kumavis Date: Tue, 2 Jan 2018 14:22:44 -0800 Subject: [PATCH 17/18] tx-gas-utils - tx-param-validation - use more intuitive check --- app/scripts/lib/tx-gas-utils.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/scripts/lib/tx-gas-utils.js b/app/scripts/lib/tx-gas-utils.js index c00323a64..bc891fc07 100644 --- a/app/scripts/lib/tx-gas-utils.js +++ b/app/scripts/lib/tx-gas-utils.js @@ -83,11 +83,11 @@ module.exports = class txProvideUtil { async validateTxParams (txParams) { if ('value' in txParams) { const value = txParams.value.toString() - if (value.indexOf('-') === 0) { + if (value.includes('-')) { throw new Error(`Invalid transaction value of ${txParams.value} not a positive number.`) } - if (value.indexOf('.') >= 0) { + if (value.includes('.')) { throw new Error(`Invalid transaction value of ${txParams.value} number must be in wei`) } } From 099f078a3d73b2aed30dc5e1cd3a2facde58606a Mon Sep 17 00:00:00 2001 From: Alexander Tseung Date: Wed, 3 Jan 2018 11:16:46 -0800 Subject: [PATCH 18/18] Fix merge conflict --- old-ui/app/send.js | 1 - 1 file changed, 1 deletion(-) diff --git a/old-ui/app/send.js b/old-ui/app/send.js index 0dc0fa778..b40910236 100644 --- a/old-ui/app/send.js +++ b/old-ui/app/send.js @@ -267,7 +267,6 @@ SendTransactionScreen.prototype.onSubmit = function () { const value = util.normalizeEthStringToWei(input) const txData = document.querySelector('input[name="txData"]').value const balance = this.props.balance - let message if (value.gt(balance)) { message = 'Insufficient funds.'