From e69b7f66a3a530812a01e6ba9b05190c4066d4e6 Mon Sep 17 00:00:00 2001 From: Jaco Greeff Date: Fri, 18 Nov 2016 14:18:50 +0100 Subject: [PATCH 1/4] Gas calculation & limit errors --- .../DetailsStep/detailsStep.js | 16 +++++ .../ExecuteContract/executeContract.css | 8 +++ .../modals/ExecuteContract/executeContract.js | 72 +++++++++++++++++-- js/src/modals/Transfer/errors.js | 4 +- js/src/modals/Transfer/transfer.css | 9 +++ js/src/modals/Transfer/transfer.js | 57 +++++++++++++-- js/src/util/constants.js | 26 +++++++ 7 files changed, 180 insertions(+), 12 deletions(-) create mode 100644 js/src/util/constants.js diff --git a/js/src/modals/ExecuteContract/DetailsStep/detailsStep.js b/js/src/modals/ExecuteContract/DetailsStep/detailsStep.js index b549abb18..aad54e360 100644 --- a/js/src/modals/ExecuteContract/DetailsStep/detailsStep.js +++ b/js/src/modals/ExecuteContract/DetailsStep/detailsStep.js @@ -36,6 +36,7 @@ export default class DetailsStep extends Component { onFuncChange: PropTypes.func, values: PropTypes.array.isRequired, valuesError: PropTypes.array.isRequired, + warning: PropTypes.string, onValueChange: PropTypes.func.isRequired } @@ -44,6 +45,7 @@ export default class DetailsStep extends Component { return (
+ { this.renderWarning() } + { warning } + + ); + } + onFuncChange = (event, index, signature) => { const { contract, onFuncChange } = this.props; diff --git a/js/src/modals/ExecuteContract/executeContract.css b/js/src/modals/ExecuteContract/executeContract.css index cdebfef32..55135a92e 100644 --- a/js/src/modals/ExecuteContract/executeContract.css +++ b/js/src/modals/ExecuteContract/executeContract.css @@ -33,3 +33,11 @@ .txhash { word-break: break-all; } + +.warning { + border-radius: 0.5em; + background: #f80; + color: white; + padding: 0.75em; + text-align: center; +} diff --git a/js/src/modals/ExecuteContract/executeContract.js b/js/src/modals/ExecuteContract/executeContract.js index a57c18a1d..8022690f5 100644 --- a/js/src/modals/ExecuteContract/executeContract.js +++ b/js/src/modals/ExecuteContract/executeContract.js @@ -15,17 +15,21 @@ // along with Parity. If not, see . import React, { Component, PropTypes } from 'react'; +import { connect } from 'react-redux'; +import { bindActionCreators } from 'redux'; import ActionDoneAll from 'material-ui/svg-icons/action/done-all'; import ContentClear from 'material-ui/svg-icons/content/clear'; import { BusyStep, CompletedStep, Button, IdentityIcon, Modal, TxHash } from '../../ui'; +import { MAX_GAS_ESTIMATION } from '../../util/constants'; import { validateAddress, validateUint } from '../../util/validation'; import DetailsStep from './DetailsStep'; +import ERRORS from '../Transfer/errors'; import { ERROR_CODES } from '../../api/transport/error'; -export default class ExecuteContract extends Component { +class ExecuteContract extends Component { static contextTypes = { api: PropTypes.object.isRequired, store: PropTypes.object.isRequired @@ -36,6 +40,7 @@ export default class ExecuteContract extends Component { fromAddress: PropTypes.string, accounts: PropTypes.object, contract: PropTypes.object, + gasLimit: PropTypes.object.isRequired, onClose: PropTypes.func.isRequired, onFromAddressChange: PropTypes.func.isRequired } @@ -46,6 +51,8 @@ export default class ExecuteContract extends Component { fromAddressError: null, func: null, funcError: null, + gas: null, + gasLimitError: null, values: [], valuesError: [], step: 0, @@ -119,7 +126,7 @@ export default class ExecuteContract extends Component { renderStep () { const { onFromAddressChange } = this.props; - const { step, busyState, txhash, rejected } = this.state; + const { step, busyState, gasLimitError, txhash, rejected } = this.state; if (rejected) { return ( @@ -135,6 +142,7 @@ export default class ExecuteContract extends Component { { + this.estimateGas(); this.setState({ amount }); } @@ -179,6 +188,7 @@ export default class ExecuteContract extends Component { } }); + this.estimateGas(); this.setState({ func, values @@ -208,17 +218,54 @@ export default class ExecuteContract extends Component { values[index] = value; valuesError[index] = valueError; + if (!valueError) { + this.estimateGas(); + } + this.setState({ values: [].concat(values), valuesError: [].concat(valuesError) }); } + estimateGas = () => { + const { api } = this.context; + const { fromAddress, gasLimit } = this.props; + const { amount, func, values } = this.state; + const options = { + gas: MAX_GAS_ESTIMATION, + from: fromAddress, + value: api.util.toWei(amount || 0) + }; + + func + .estimateGas(options, values) + .then((gasEst) => { + const gas = gasEst.mul(1.2); + let gasLimitError = null; + + if (gas.gte(MAX_GAS_ESTIMATION)) { + gasLimitError = ERRORS.gasException; + } else if (gas.gt(gasLimit)) { + gasLimitError = ERRORS.gasBlockLimit; + } + + this.setState({ + gas, + gasLimitError + }); + }) + .catch((error) => { + console.warn('estimateGas', error); + }); + } + postTransaction = () => { const { api, store } = this.context; const { fromAddress } = this.props; const { amount, func, values } = this.state; const options = { + gas: MAX_GAS_ESTIMATION, from: fromAddress, value: api.util.toWei(amount || 0) }; @@ -237,13 +284,13 @@ export default class ExecuteContract extends Component { return api .pollMethod('parity_checkRequest', requestId) - .catch((e) => { - if (e.code === ERROR_CODES.REQUEST_REJECTED) { + .catch((error) => { + if (error.code === ERROR_CODES.REQUEST_REJECTED) { this.setState({ rejected: true }); return false; } - throw e; + throw error; }); }) .then((txhash) => { @@ -255,3 +302,18 @@ export default class ExecuteContract extends Component { }); } } + +function mapStateToProps (state) { + const { gasLimit } = state.status; + + return { gasLimit }; +} + +function mapDispatchToProps (dispatch) { + return bindActionCreators({}, dispatch); +} + +export default connect( + mapStateToProps, + mapDispatchToProps +)(ExecuteContract); diff --git a/js/src/modals/Transfer/errors.js b/js/src/modals/Transfer/errors.js index a6456c785..c35d82636 100644 --- a/js/src/modals/Transfer/errors.js +++ b/js/src/modals/Transfer/errors.js @@ -19,7 +19,9 @@ const ERRORS = { invalidAddress: 'the supplied address is an invalid network address', invalidAmount: 'the supplied amount should be a valid positive number', invalidDecimals: 'the supplied amount exceeds the allowed decimals', - largeAmount: 'the transaction total is higher than the available balance' + largeAmount: 'the transaction total is higher than the available balance', + gasException: 'the transaction indicates a thrown exception', + gasBlockLimit: 'the transaction will exceed the block gas limit' }; export default ERRORS; diff --git a/js/src/modals/Transfer/transfer.css b/js/src/modals/Transfer/transfer.css index 89b58666e..76f83c62b 100644 --- a/js/src/modals/Transfer/transfer.css +++ b/js/src/modals/Transfer/transfer.css @@ -14,6 +14,7 @@ /* You should have received a copy of the GNU General Public License /* along with Parity. If not, see . */ + .info { line-height: 1.618em; width: 100%; @@ -151,3 +152,11 @@ .gasPriceDesc { font-size: 0.9em; } + +.warning { + border-radius: 0.5em; + background: #f80; + color: white; + padding: 0.75em; + text-align: center; +} diff --git a/js/src/modals/Transfer/transfer.js b/js/src/modals/Transfer/transfer.js index e41ea08f9..196307923 100644 --- a/js/src/modals/Transfer/transfer.js +++ b/js/src/modals/Transfer/transfer.js @@ -16,12 +16,15 @@ import BigNumber from 'bignumber.js'; import React, { Component, PropTypes } from 'react'; +import { connect } from 'react-redux'; +import { bindActionCreators } from 'redux'; import ActionDoneAll from 'material-ui/svg-icons/action/done-all'; import ContentClear from 'material-ui/svg-icons/content/clear'; import NavigationArrowBack from 'material-ui/svg-icons/navigation/arrow-back'; import NavigationArrowForward from 'material-ui/svg-icons/navigation/arrow-forward'; import { BusyStep, CompletedStep, Button, IdentityIcon, Modal, TxHash } from '../../ui'; +import { DEFAULT_GAS, DEFAULT_GASPRICE, MAX_GAS_ESTIMATION } from '../../util/constants'; import Details from './Details'; import Extras from './Extras'; @@ -30,8 +33,6 @@ import styles from './transfer.css'; import { ERROR_CODES } from '../../api/transport/error'; -const DEFAULT_GAS = '21000'; -const DEFAULT_GASPRICE = '20000000000'; const TITLES = { transfer: 'transfer details', sending: 'sending', @@ -42,7 +43,7 @@ const TITLES = { const STAGES_BASIC = [TITLES.transfer, TITLES.sending, TITLES.complete]; const STAGES_EXTRA = [TITLES.transfer, TITLES.extras, TITLES.sending, TITLES.complete]; -export default class Transfer extends Component { +class Transfer extends Component { static contextTypes = { api: PropTypes.object.isRequired, store: PropTypes.object.isRequired @@ -52,6 +53,7 @@ export default class Transfer extends Component { account: PropTypes.object, balance: PropTypes.object, balances: PropTypes.object, + gasLimit: PropTypes.object.isRequired, images: PropTypes.object.isRequired, onClose: PropTypes.func } @@ -64,6 +66,7 @@ export default class Transfer extends Component { gas: DEFAULT_GAS, gasEst: '0', gasError: null, + gasLimitError: null, gasPrice: DEFAULT_GASPRICE, gasPriceHistogram: {}, gasPriceError: null, @@ -103,6 +106,7 @@ export default class Transfer extends Component { visible scroll > + { this.renderWarning() } { this.renderPage() } ); @@ -264,6 +268,20 @@ export default class Transfer extends Component { } } + renderWarning () { + const { gasLimitError } = this.state; + + if (!gasLimitError) { + return null; + } + + return ( +
+ { gasLimitError } +
+ ); + } + isValid () { const detailsValid = !this.state.recipientError && !this.state.valueError && !this.state.totalError; const extrasValid = !this.state.gasError && !this.state.gasPriceError && !this.state.totalError; @@ -519,6 +537,7 @@ export default class Transfer extends Component { return token.contract.instance.transfer .estimateGas({ + gas: MAX_GAS_ESTIMATION, from: account.address, to: token.address }, [ @@ -532,6 +551,7 @@ export default class Transfer extends Component { const { account } = this.props; const { data, recipient, value } = this.state; const options = { + gas: MAX_GAS_ESTIMATION, from: account.address, to: recipient, value: api.util.toWei(value || 0) @@ -552,19 +572,29 @@ export default class Transfer extends Component { return; } + const { gasLimit } = this.props; + (this.state.isEth ? this._estimateGasEth() : this._estimateGasToken() - ).then((_value) => { - let gas = _value; + ).then((gasEst) => { + let gas = gasEst; + let gasLimitError = null; if (gas.gt(DEFAULT_GAS)) { gas = gas.mul(1.2); } + if (gas.gte(MAX_GAS_ESTIMATION)) { + gasLimitError = ERRORS.gasException; + } else if (gas.gt(gasLimit)) { + gasLimitError = ERRORS.gasBlockLimit; + } + this.setState({ gas: gas.toFixed(0), - gasEst: _value.toFormat() + gasEst: gasEst.toFormat(), + gasLimitError }, this.recalculate); }) .catch((error) => { @@ -649,3 +679,18 @@ export default class Transfer extends Component { store.dispatch({ type: 'newError', error }); } } + +function mapStateToProps (state) { + const { gasLimit } = state.status; + + return { gasLimit }; +} + +function mapDispatchToProps (dispatch) { + return bindActionCreators({}, dispatch); +} + +export default connect( + mapStateToProps, + mapDispatchToProps +)(Transfer); diff --git a/js/src/util/constants.js b/js/src/util/constants.js new file mode 100644 index 000000000..87ca59668 --- /dev/null +++ b/js/src/util/constants.js @@ -0,0 +1,26 @@ +// Copyright 2015, 2016 Ethcore (UK) Ltd. +// This file is part of Parity. + +// Parity is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Parity is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Parity. If not, see . + +const DEFAULT_GAS = '21000'; +const DEFAULT_GASPRICE = '20000000000'; + +const MAX_GAS_ESTIMATION = '50000000'; + +export default { + DEFAULT_GAS, + DEFAULT_GASPRICE, + MAX_GAS_ESTIMATION +}; From b433e7e9a0207b6d3c6b7403ea9c54dee6787fc9 Mon Sep 17 00:00:00 2001 From: Jaco Greeff Date: Fri, 18 Nov 2016 15:05:02 +0100 Subject: [PATCH 2/4] Updated, contract execution in-place --- .../ExecuteContract/executeContract.css | 1 + .../modals/ExecuteContract/executeContract.js | 31 ++++++++++++------- js/src/modals/Transfer/errors.js | 4 +-- js/src/modals/Transfer/transfer.css | 1 + js/src/modals/Transfer/transfer.js | 2 +- js/src/util/constants.js | 2 +- 6 files changed, 26 insertions(+), 15 deletions(-) diff --git a/js/src/modals/ExecuteContract/executeContract.css b/js/src/modals/ExecuteContract/executeContract.css index 55135a92e..a83b373ee 100644 --- a/js/src/modals/ExecuteContract/executeContract.css +++ b/js/src/modals/ExecuteContract/executeContract.css @@ -38,6 +38,7 @@ border-radius: 0.5em; background: #f80; color: white; + font-size: 0.75em; padding: 0.75em; text-align: center; } diff --git a/js/src/modals/ExecuteContract/executeContract.js b/js/src/modals/ExecuteContract/executeContract.js index 8022690f5..1354e8543 100644 --- a/js/src/modals/ExecuteContract/executeContract.js +++ b/js/src/modals/ExecuteContract/executeContract.js @@ -71,6 +71,12 @@ class ExecuteContract extends Component { this.onFuncChange(null, functions[0]); } + componentWillReceiveProps (newProps) { + if (newProps.fromAddress !== this.props.fromAddress) { + this.estimateGas(newProps.fromAddress); + } + } + render () { const { sending } = this.state; @@ -164,8 +170,7 @@ class ExecuteContract extends Component { } onAmountChange = (amount) => { - this.estimateGas(); - this.setState({ amount }); + this.setState({ amount }, this.estimateGas); } onFuncChange = (event, func) => { @@ -188,11 +193,10 @@ class ExecuteContract extends Component { } }); - this.estimateGas(); this.setState({ func, values - }); + }, this.estimateGas); } onValueChange = (event, index, _value) => { @@ -218,29 +222,34 @@ class ExecuteContract extends Component { values[index] = value; valuesError[index] = valueError; - if (!valueError) { - this.estimateGas(); - } - this.setState({ values: [].concat(values), valuesError: [].concat(valuesError) + }, () => { + if (!valueError) { + this.estimateGas(); + } }); } - estimateGas = () => { + estimateGas = (_fromAddress) => { const { api } = this.context; const { fromAddress, gasLimit } = this.props; const { amount, func, values } = this.state; const options = { gas: MAX_GAS_ESTIMATION, - from: fromAddress, + from: _fromAddress || fromAddress, value: api.util.toWei(amount || 0) }; + if (!func) { + return; + } + func .estimateGas(options, values) .then((gasEst) => { + console.log(gasEst.toFormat(0)); const gas = gasEst.mul(1.2); let gasLimitError = null; @@ -304,7 +313,7 @@ class ExecuteContract extends Component { } function mapStateToProps (state) { - const { gasLimit } = state.status; + const { gasLimit } = state.nodeStatus; return { gasLimit }; } diff --git a/js/src/modals/Transfer/errors.js b/js/src/modals/Transfer/errors.js index c35d82636..3a6bd63ae 100644 --- a/js/src/modals/Transfer/errors.js +++ b/js/src/modals/Transfer/errors.js @@ -20,8 +20,8 @@ const ERRORS = { invalidAmount: 'the supplied amount should be a valid positive number', invalidDecimals: 'the supplied amount exceeds the allowed decimals', largeAmount: 'the transaction total is higher than the available balance', - gasException: 'the transaction indicates a thrown exception', - gasBlockLimit: 'the transaction will exceed the block gas limit' + gasException: 'the transaction will throw an exception with the current values', + gasBlockLimit: 'the transaction execution will exceed the block gas limit' }; export default ERRORS; diff --git a/js/src/modals/Transfer/transfer.css b/js/src/modals/Transfer/transfer.css index 76f83c62b..b7f478b4f 100644 --- a/js/src/modals/Transfer/transfer.css +++ b/js/src/modals/Transfer/transfer.css @@ -157,6 +157,7 @@ border-radius: 0.5em; background: #f80; color: white; + font-size: 0.75em; padding: 0.75em; text-align: center; } diff --git a/js/src/modals/Transfer/transfer.js b/js/src/modals/Transfer/transfer.js index 196307923..e906e45d7 100644 --- a/js/src/modals/Transfer/transfer.js +++ b/js/src/modals/Transfer/transfer.js @@ -681,7 +681,7 @@ class Transfer extends Component { } function mapStateToProps (state) { - const { gasLimit } = state.status; + const { gasLimit } = state.nodeStatus; return { gasLimit }; } diff --git a/js/src/util/constants.js b/js/src/util/constants.js index 87ca59668..d0664d25e 100644 --- a/js/src/util/constants.js +++ b/js/src/util/constants.js @@ -19,7 +19,7 @@ const DEFAULT_GASPRICE = '20000000000'; const MAX_GAS_ESTIMATION = '50000000'; -export default { +export { DEFAULT_GAS, DEFAULT_GASPRICE, MAX_GAS_ESTIMATION From 8d339ad379239f1968693952f39c3340dbbd8bf2 Mon Sep 17 00:00:00 2001 From: Jaco Greeff Date: Fri, 18 Nov 2016 15:13:37 +0100 Subject: [PATCH 3/4] Adjust styling --- js/src/modals/Transfer/transfer.css | 1 + 1 file changed, 1 insertion(+) diff --git a/js/src/modals/Transfer/transfer.css b/js/src/modals/Transfer/transfer.css index b7f478b4f..3bd17ba96 100644 --- a/js/src/modals/Transfer/transfer.css +++ b/js/src/modals/Transfer/transfer.css @@ -158,6 +158,7 @@ background: #f80; color: white; font-size: 0.75em; + margin-bottom: 1em; padding: 0.75em; text-align: center; } From f827ade6161caca36ec7cecffb7e1eba2b0da762 Mon Sep 17 00:00:00 2001 From: Jaco Greeff Date: Fri, 18 Nov 2016 16:32:57 +0100 Subject: [PATCH 4/4] debug log removal --- js/src/modals/ExecuteContract/executeContract.js | 1 - 1 file changed, 1 deletion(-) diff --git a/js/src/modals/ExecuteContract/executeContract.js b/js/src/modals/ExecuteContract/executeContract.js index 1354e8543..97d44a029 100644 --- a/js/src/modals/ExecuteContract/executeContract.js +++ b/js/src/modals/ExecuteContract/executeContract.js @@ -249,7 +249,6 @@ class ExecuteContract extends Component { func .estimateGas(options, values) .then((gasEst) => { - console.log(gasEst.toFormat(0)); const gas = gasEst.mul(1.2); let gasLimitError = null;