From dc10b21b2eff6826a32291b6a7398eaae2ec9efa Mon Sep 17 00:00:00 2001 From: yann300 Date: Tue, 13 Mar 2018 13:59:35 +0100 Subject: [PATCH 1/2] warn if gas estimation fails and propose to force --- src/app/execution/txRunner.js | 73 ++++++++++++++++++----------------- src/universal-dapp.js | 22 ++++++++++- 2 files changed, 59 insertions(+), 36 deletions(-) diff --git a/src/app/execution/txRunner.js b/src/app/execution/txRunner.js index 5c7c8392b5..7783c08a80 100644 --- a/src/app/execution/txRunner.js +++ b/src/app/execution/txRunner.js @@ -19,8 +19,8 @@ function TxRunner (vmaccounts, api) { this.queusTxs = [] } -TxRunner.prototype.rawRun = function (args, confirmationCb, cb) { - run(this, args, Date.now(), confirmationCb, cb) +TxRunner.prototype.rawRun = function (args, confirmationCb, gasEstimationForceSend, cb) { + run(this, args, Date.now(), confirmationCb, gasEstimationForceSend, cb) } function executeTx (tx, gasPrice, api, callback) { @@ -37,7 +37,7 @@ function executeTx (tx, gasPrice, api, callback) { } } -TxRunner.prototype.execute = function (args, confirmationCb, callback) { +TxRunner.prototype.execute = function (args, confirmationCb, gasEstimationForceSend, callback) { var self = this var data = args.data @@ -46,7 +46,7 @@ TxRunner.prototype.execute = function (args, confirmationCb, callback) { } if (!executionContext.isVM()) { - self.runInNode(args.from, args.to, data, args.value, args.gasLimit, args.useCall, confirmationCb, callback) + self.runInNode(args.from, args.to, data, args.value, args.gasLimit, args.useCall, confirmationCb, gasEstimationForceSend, callback) } else { try { self.runInVm(args.from, args.to, data, args.value, args.gasLimit, args.useCall, callback) @@ -104,7 +104,7 @@ TxRunner.prototype.runInVm = function (from, to, data, value, gasLimit, useCall, }) } -TxRunner.prototype.runInNode = function (from, to, data, value, gasLimit, useCall, confirmCb, callback) { +TxRunner.prototype.runInNode = function (from, to, data, value, gasLimit, useCall, confirmCb, gasEstimationForceSend, callback) { const self = this var tx = { from: from, to: to, data: data, value: value } @@ -118,38 +118,41 @@ TxRunner.prototype.runInNode = function (from, to, data, value, gasLimit, useCal }) } executionContext.web3().eth.estimateGas(tx, function (err, gasEstimation) { - if (err) { - return callback(err, gasEstimation) - } - var blockGasLimit = executionContext.currentblockGasLimit() - // NOTE: estimateGas very likely will return a large limit if execution of the code failed - // we want to be able to run the code in order to debug and find the cause for the failure + gasEstimationForceSend(err, () => { + // callback is called whenever no error + tx.gas = !gasEstimation ? gasLimit : gasEstimation - var warnEstimation = " An important gas estimation might also be the sign of a problem in the contract code. Please check loops and be sure you did not sent value to a non payable function (that's also the reason of strong gas estimation)." - if (gasEstimation > gasLimit) { - return callback('Gas required exceeds limit: ' + gasLimit + '. ' + warnEstimation) - } - if (gasEstimation > blockGasLimit) { - return callback('Gas required exceeds block gas limit: ' + gasLimit + '. ' + warnEstimation) - } + if (self._api.config.getUnpersistedProperty('doNotShowTransactionConfirmationAgain')) { + return executeTx(tx, null, self._api, callback) + } - tx.gas = gasEstimation + self._api.detectNetwork((err, network) => { + if (err) { + console.log(err) + return + } + + confirmCb(network, tx, tx.gas, (gasPrice) => { + return executeTx(tx, gasPrice, self._api, callback) + }, (error) => { + callback(error) + }) + }) + }, () => { + var blockGasLimit = executionContext.currentblockGasLimit() + // NOTE: estimateGas very likely will return a large limit if execution of the code failed + // we want to be able to run the code in order to debug and find the cause for the failure + if (err) return callback(err) - if (self._api.config.getUnpersistedProperty('doNotShowTransactionConfirmationAgain')) { - return executeTx(tx, null, self._api, callback) - } + var warnEstimation = ' An important gas estimation might also be the sign of a problem in the contract code. Please check loops and be sure you did not sent value to a non payable function (that\'s also the reason of strong gas estimation). ' + warnEstimation += ' ' + err - self._api.detectNetwork((err, network) => { - if (err) { - console.log(err) - return + if (gasEstimation > gasLimit) { + return callback('Gas required exceeds limit: ' + gasLimit + '. ' + warnEstimation) + } + if (gasEstimation > blockGasLimit) { + return callback('Gas required exceeds block gas limit: ' + gasLimit + '. ' + warnEstimation) } - - confirmCb(network, tx, gasEstimation, (gasPrice) => { - return executeTx(tx, gasPrice, self._api, callback) - }, (error) => { - callback(error) - }) }) }) } @@ -183,12 +186,12 @@ function sendTransaction (sendTx, tx, pass, callback) { } } -function run (self, tx, stamp, confirmationCb, callback) { +function run (self, tx, stamp, confirmationCb, gasEstimationForceSend, callback) { if (!self.runAsync && Object.keys(self.pendingTxs).length) { - self.queusTxs.push({ tx, stamp, callback }) + self.queusTxs.push({ tx, stamp, callback}) } else { self.pendingTxs[stamp] = tx - self.execute(tx, confirmationCb, (error, result) => { + self.execute(tx, confirmationCb, gasEstimationForceSend, (error, result) => { delete self.pendingTxs[stamp] callback(error, result) if (self.queusTxs.length) { diff --git a/src/universal-dapp.js b/src/universal-dapp.js index cb81ff2381..cb648409d3 100644 --- a/src/universal-dapp.js +++ b/src/universal-dapp.js @@ -1,6 +1,7 @@ /* global */ 'use strict' +var yo = require('yo-yo') var async = require('async') var ethJSUtil = require('ethereumjs-util') var BN = ethJSUtil.BN @@ -308,7 +309,26 @@ UniversalDApp.prototype.runTx = function (args, cb) { } }) }, - + (error, continueTxExecution, cancelCb) => { + if (error) { + var msg = typeof error !== 'string' ? error.message : error + modalDialog('Gas estimation failed', yo`
Gas estimation errored with the following message (see below). + The transaction execution will likely fail. Do you want to force sending?
+ ${msg} +
`, + { label: 'Send Transaction', + fn: () => { + continueTxExecution() + }}, { + label: 'Cancel Transaction', + fn: () => { + cancelCb() + } + }) + } else { + continueTxExecution() + } + }, function (error, result) { let eventName = (tx.useCall ? 'callExecuted' : 'transactionExecuted') self.event.trigger(eventName, [error, tx.from, tx.to, tx.data, tx.useCall, result, timestamp, payLoad]) From 21836221d0cd07323bf6d58c91244c643ce80bc5 Mon Sep 17 00:00:00 2001 From: yann300 Date: Tue, 13 Mar 2018 14:06:36 +0100 Subject: [PATCH 2/2] standard --- src/app/execution/txRunner.js | 2 +- src/universal-dapp.js | 17 +++++++++-------- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/app/execution/txRunner.js b/src/app/execution/txRunner.js index 7783c08a80..6ad1fa4a97 100644 --- a/src/app/execution/txRunner.js +++ b/src/app/execution/txRunner.js @@ -188,7 +188,7 @@ function sendTransaction (sendTx, tx, pass, callback) { function run (self, tx, stamp, confirmationCb, gasEstimationForceSend, callback) { if (!self.runAsync && Object.keys(self.pendingTxs).length) { - self.queusTxs.push({ tx, stamp, callback}) + self.queusTxs.push({ tx, stamp, callback }) } else { self.pendingTxs[stamp] = tx self.execute(tx, confirmationCb, gasEstimationForceSend, (error, result) => { diff --git a/src/universal-dapp.js b/src/universal-dapp.js index cb648409d3..e84f10810e 100644 --- a/src/universal-dapp.js +++ b/src/universal-dapp.js @@ -316,15 +316,16 @@ UniversalDApp.prototype.runTx = function (args, cb) { The transaction execution will likely fail. Do you want to force sending?
${msg} `, - { label: 'Send Transaction', - fn: () => { - continueTxExecution() - }}, { - label: 'Cancel Transaction', + { + label: 'Send Transaction', fn: () => { - cancelCb() - } - }) + continueTxExecution() + }}, { + label: 'Cancel Transaction', + fn: () => { + cancelCb() + } + }) } else { continueTxExecution() }