From 06153dd47d16c2e3c6eed7470501d7534c29cbd4 Mon Sep 17 00:00:00 2001 From: Kevin Serrano Date: Wed, 13 Sep 2017 12:17:42 -0700 Subject: [PATCH 1/4] Add warning of higher failure risk since app proposed gasLimit. --- ui/app/components/pending-tx.js | 64 ++++++++++++++++++++------------- 1 file changed, 39 insertions(+), 25 deletions(-) diff --git a/ui/app/components/pending-tx.js b/ui/app/components/pending-tx.js index 3e53d47f9..37b242728 100644 --- a/ui/app/components/pending-tx.js +++ b/ui/app/components/pending-tx.js @@ -52,7 +52,8 @@ PendingTx.prototype.render = function () { const gas = txParams.gas const gasBn = hexToBn(gas) const gasLimit = new BN(parseInt(blockGasLimit)) - const safeGasLimit = this.bnMultiplyByFraction(gasLimit, 19, 20).toString(10) + const safeGasLimitBN = this.bnMultiplyByFraction(gasLimit, 19, 20) + const safeGasLimit = safeGasLimitBN.toString(10) // Gas Price const gasPrice = txParams.gasPrice || MIN_GAS_PRICE_BN.toString(16) @@ -66,6 +67,8 @@ PendingTx.prototype.render = function () { const balanceBn = hexToBn(balance) const insufficientBalance = balanceBn.lt(maxCost) + const dangerousGasLimit = gasBn.gte(safeGasLimitBN) + const gasLimitSpecified = txMeta.gasLimitSpecified const buyDisabled = insufficientBalance || !this.state.valid || !isValidAddress || this.state.submitting const showRejectAll = props.unconfTxListLength > 1 @@ -263,33 +266,44 @@ PendingTx.prototype.render = function () { text-transform: uppercase; } `), + h('.cell.row', { + style: { + textAlign: 'center', + }, + }, [ + txMeta.simulationFails ? + h('.error', { + style: { + fontSize: '0.9em', + }, + }, 'Transaction Error. Exception thrown in contract code.') + : null, - txMeta.simulationFails ? - h('.error', { - style: { - marginLeft: 50, - fontSize: '0.9em', - }, - }, 'Transaction Error. Exception thrown in contract code.') - : null, + !isValidAddress ? + h('.error', { + style: { + fontSize: '0.9em', + }, + }, 'Recipient address is invalid. Sending this transaction will result in a loss of ETH.') + : null, - !isValidAddress ? - h('.error', { - style: { - marginLeft: 50, - fontSize: '0.9em', - }, - }, 'Recipient address is invalid. Sending this transaction will result in a loss of ETH.') - : null, + insufficientBalance ? + h('span.error', { + style: { + fontSize: '0.9em', + }, + }, 'Insufficient balance for transaction') + : null, + + (dangerousGasLimit && gasLimitSpecified) ? + h('span.error', { + style: { + fontSize: '0.9em', + }, + }, 'Gas limit set dangerously high. Approving this transaction is likely to fail.') + : null, + ]), - insufficientBalance ? - h('span.error', { - style: { - marginLeft: 50, - fontSize: '0.9em', - }, - }, 'Insufficient balance for transaction') - : null, // send + cancel h('.flex-row.flex-space-around.conf-buttons', { From 6e725b123b878da12494904b121237ee41af7f76 Mon Sep 17 00:00:00 2001 From: Kevin Serrano Date: Wed, 13 Sep 2017 12:22:08 -0700 Subject: [PATCH 2/4] Lower warning threshold for high tx fee to account for fluctuating blockGasLimits --- ui/app/components/pending-tx.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ui/app/components/pending-tx.js b/ui/app/components/pending-tx.js index 37b242728..ec2b15de4 100644 --- a/ui/app/components/pending-tx.js +++ b/ui/app/components/pending-tx.js @@ -53,6 +53,7 @@ PendingTx.prototype.render = function () { const gasBn = hexToBn(gas) const gasLimit = new BN(parseInt(blockGasLimit)) const safeGasLimitBN = this.bnMultiplyByFraction(gasLimit, 19, 20) + const saferGasLimitBN = this.bnMultiplyByFraction(gasLimit, 18, 20) const safeGasLimit = safeGasLimitBN.toString(10) // Gas Price @@ -67,7 +68,7 @@ PendingTx.prototype.render = function () { const balanceBn = hexToBn(balance) const insufficientBalance = balanceBn.lt(maxCost) - const dangerousGasLimit = gasBn.gte(safeGasLimitBN) + const dangerousGasLimit = gasBn.gte(saferGasLimitBN) const gasLimitSpecified = txMeta.gasLimitSpecified const buyDisabled = insufficientBalance || !this.state.valid || !isValidAddress || this.state.submitting const showRejectAll = props.unconfTxListLength > 1 From 3d36d565afe908ca7c4424ff8a7d3659d0978aca Mon Sep 17 00:00:00 2001 From: Kevin Serrano Date: Wed, 13 Sep 2017 12:26:24 -0700 Subject: [PATCH 3/4] Bump changelog. --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f30c04985..6f357c92c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ - Add ability to export seed words as a file. - Changed state logs to a file download than a clipboard copy. - Fix link to support center. +- Warn users when a dapp proposes a high gas limit (90% of blockGasLimit or higher) ## 3.10.0 2017-9-11 From a22a2586abcca35d2a4bc1a0407aeaacc55f0a27 Mon Sep 17 00:00:00 2001 From: Kevin Serrano Date: Wed, 13 Sep 2017 13:24:16 -0700 Subject: [PATCH 4/4] Haha silly me, only when gas is estimated and not explicit. --- ui/app/components/pending-tx.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/app/components/pending-tx.js b/ui/app/components/pending-tx.js index ec2b15de4..c3350fcc1 100644 --- a/ui/app/components/pending-tx.js +++ b/ui/app/components/pending-tx.js @@ -296,7 +296,7 @@ PendingTx.prototype.render = function () { }, 'Insufficient balance for transaction') : null, - (dangerousGasLimit && gasLimitSpecified) ? + (dangerousGasLimit && !gasLimitSpecified) ? h('span.error', { style: { fontSize: '0.9em',