From 632c9b21e297f3255ecbb4a70812aa53c87b2ff4 Mon Sep 17 00:00:00 2001 From: Dan J Miller Date: Wed, 3 Jul 2019 18:03:44 -0230 Subject: Version 6.7.2 gas limit fix (#6786) * Introduce delay for eth_estimateGas calls with in test * Add test that fails when gas estimates of contract method calls without gas are too high. * Get transaction gas data from unApprovedTxs instead of confirmTransaction * Fix selection of gas data in gas-modal-page-container.container * Lint changes related to Version-6.7.2-gasLimitFix * Fix e2e tests on Version-6.7.2-gasLimitFix * Fix unit and integration tests for changes from Version-6.7.2-gasLimitFix * more e2e fixes * Add assertions for transaction values on confirm screen * Fix display of transaction amount on confirm screen. --- .../advanced-gas-inputs.component.js | 63 ++++++++++++++++------ .../gas-modal-page-container.container.js | 41 ++++++++------ .../gas-modal-page-container-container.test.js | 23 ++++++-- 3 files changed, 93 insertions(+), 34 deletions(-) (limited to 'ui/app/components') diff --git a/ui/app/components/app/gas-customization/advanced-gas-inputs/advanced-gas-inputs.component.js b/ui/app/components/app/gas-customization/advanced-gas-inputs/advanced-gas-inputs.component.js index d6c259033..d942fd150 100644 --- a/ui/app/components/app/gas-customization/advanced-gas-inputs/advanced-gas-inputs.component.js +++ b/ui/app/components/app/gas-customization/advanced-gas-inputs/advanced-gas-inputs.component.js @@ -8,6 +8,17 @@ export default class AdvancedTabContent extends Component { t: PropTypes.func, } + constructor (props) { + super(props) + this.state = { + gasPrice: this.props.customGasPrice, + gasLimit: this.props.customGasLimit, + } + this.changeGasPrice = debounce(this.changeGasPrice, 500) + this.changeGasLimit = debounce(this.changeGasLimit, 500) + } + + static propTypes = { updateCustomGasPrice: PropTypes.func, updateCustomGasLimit: PropTypes.func, @@ -20,15 +31,40 @@ export default class AdvancedTabContent extends Component { showGasLimitInfoModal: PropTypes.func, } - debouncedGasLimitReset = debounce((dVal) => { - if (dVal < 21000) { + componentDidUpdate (prevProps) { + const { customGasPrice: prevCustomGasPrice, customGasLimit: prevCustomGasLimit } = prevProps + const { customGasPrice, customGasLimit } = this.props + const { gasPrice, gasLimit } = this.state + + if (customGasPrice !== prevCustomGasPrice && customGasPrice !== gasPrice) { + this.setState({ gasPrice: customGasPrice }) + } + if (customGasLimit !== prevCustomGasLimit && customGasLimit !== gasLimit) { + this.setState({ gasLimit: customGasLimit }) + } + } + + onChangeGasLimit = (e) => { + this.setState({ gasLimit: e.target.value }) + this.changeGasLimit({ target: { value: e.target.value } }) + } + + changeGasLimit = (e) => { + if (e.target.value < 21000) { + this.setState({ gasLimit: 21000 }) this.props.updateCustomGasLimit(21000) + } else { + this.props.updateCustomGasLimit(Number(e.target.value)) } - }, 1000, { trailing: true }) + } + + onChangeGasPrice = (e) => { + this.setState({ gasPrice: e.target.value }) + this.changeGasPrice({ target: { value: e.target.value } }) + } - onChangeGasLimit = (val) => { - this.props.updateCustomGasLimit(val) - this.debouncedGasLimitReset(val) + changeGasPrice = (e) => { + this.props.updateCustomGasPrice(Number(e.target.value)) } gasInputError ({ labelKey, insufficientBalance, customPriceIsSafe, isSpeedUp, value }) { @@ -74,7 +110,7 @@ export default class AdvancedTabContent extends Component { })} type="number" value={value} - onChange={event => onChange(Number(event.target.value))} + onChange={onChange} />
onChange(value + 1)} + onClick={() => onChange({ target: { value: value + 1 } })} >
onChange(Math.max(value - 1, 0))} + onClick={() => onChange({ target: { value: Math.max(value - 1, 0) } })} >
@@ -120,9 +156,6 @@ export default class AdvancedTabContent extends Component { render () { const { - customGasPrice, - updateCustomGasPrice, - customGasLimit, insufficientBalance, customPriceIsSafe, isSpeedUp, @@ -134,8 +167,8 @@ export default class AdvancedTabContent extends Component {
{ this.renderGasEditRow({ labelKey: 'gasPrice', - value: customGasPrice, - onChange: updateCustomGasPrice, + value: this.state.gasPrice, + onChange: this.onChangeGasPrice, insufficientBalance, customPriceIsSafe, showGWEI: true, @@ -144,7 +177,7 @@ export default class AdvancedTabContent extends Component { }) } { this.renderGasEditRow({ labelKey: 'gasLimit', - value: customGasLimit, + value: this.state.gasLimit, onChange: this.onChangeGasLimit, insufficientBalance, customPriceIsSafe, diff --git a/ui/app/components/app/gas-customization/gas-modal-page-container/gas-modal-page-container.container.js b/ui/app/components/app/gas-customization/gas-modal-page-container/gas-modal-page-container.container.js index 9da9a2ef6..c260d6798 100644 --- a/ui/app/components/app/gas-customization/gas-modal-page-container/gas-modal-page-container.container.js +++ b/ui/app/components/app/gas-customization/gas-modal-page-container/gas-modal-page-container.container.js @@ -9,6 +9,7 @@ import { hideSidebar, updateSendAmount, setGasTotal, + updateTransaction, } from '../../../../store/actions' import { setCustomGasPrice, @@ -22,9 +23,6 @@ import { hideGasButtonGroup, updateSendErrors, } from '../../../../ducks/send/send.duck' -import { - updateGasAndCalculate, -} from '../../../../ducks/confirm-transaction/confirm-transaction.duck' import { conversionRateSelector as getConversionRate, getCurrentCurrency, @@ -51,9 +49,6 @@ import { import { getTokenBalance, } from '../../../../pages/send/send.selectors' -import { - submittedPendingTransactionsSelector, -} from '../../../../selectors/transactions' import { formatCurrency, } from '../../../../helpers/utils/confirm-tx.util' @@ -77,11 +72,16 @@ import { getMaxModeOn } from '../../../../pages/send/send-content/send-amount-ro import { calcMaxAmount } from '../../../../pages/send/send-content/send-amount-row/amount-max-button/amount-max-button.utils' const mapStateToProps = (state, ownProps) => { + const { selectedAddressTxList } = state.metamask + const { modalState: { props: modalProps } = {} } = state.appState.modal || {} + const { txData = {} } = modalProps || {} const { transaction = {} } = ownProps + const selectedTransaction = selectedAddressTxList.find(({ id }) => id === (transaction.id || txData.id)) + const buttonDataLoading = getBasicGasEstimateLoadingStatus(state) const gasEstimatesLoading = getGasEstimatesLoadingStatus(state) - const { gasPrice: currentGasPrice, gas: currentGasLimit, value } = getTxParams(state, transaction.id) + const { gasPrice: currentGasPrice, gas: currentGasLimit, value } = getTxParams(state, selectedTransaction) const customModalGasPriceInHex = getCustomGasPrice(state) || currentGasPrice const customModalGasLimitInHex = getCustomGasLimit(state) || currentGasLimit const customGasTotal = calcGasTotal(customModalGasLimitInHex, customModalGasPriceInHex) @@ -118,6 +118,7 @@ const mapStateToProps = (state, ownProps) => { conversionRate, }) + return { hideBasic, isConfirm: isConfirm(state), @@ -151,6 +152,7 @@ const mapStateToProps = (state, ownProps) => { transactionFee: addHexWEIsToRenderableEth('0x0', customGasTotal), sendAmount, }, + transaction: txData || transaction, isSpeedUp: transaction.status === 'submitted', txId: transaction.id, insufficientBalance, @@ -179,10 +181,10 @@ const mapDispatchToProps = dispatch => { dispatch(setGasLimit(newLimit)) dispatch(setGasPrice(newPrice)) }, - updateConfirmTxGasAndCalculate: (gasLimit, gasPrice) => { + updateConfirmTxGasAndCalculate: (gasLimit, gasPrice, updatedTx) => { updateCustomGasPrice(gasPrice) dispatch(setCustomGasLimit(addHexPrefix(gasLimit.toString(16)))) - return dispatch(updateGasAndCalculate({ gasLimit, gasPrice })) + return dispatch(updateTransaction(updatedTx)) }, createSpeedUpTransaction: (txId, gasPrice) => { return dispatch(createSpeedUpTransaction(txId, gasPrice)) @@ -214,6 +216,7 @@ const mergeProps = (stateProps, dispatchProps, ownProps) => { selectedToken, tokenBalance, customGasLimit, + transaction, } = stateProps const { updateCustomGasPrice: dispatchUpdateCustomGasPrice, @@ -234,7 +237,15 @@ const mergeProps = (stateProps, dispatchProps, ownProps) => { ...ownProps, onSubmit: (gasLimit, gasPrice) => { if (isConfirm) { - dispatchUpdateConfirmTxGasAndCalculate(gasLimit, gasPrice) + const updatedTx = { + ...transaction, + txParams: { + ...transaction.txParams, + gas: gasLimit, + gasPrice, + }, + } + dispatchUpdateConfirmTxGasAndCalculate(gasLimit, gasPrice, updatedTx) dispatchHideModal() } else if (isSpeedUp) { dispatchCreateSpeedUpTransaction(txId, gasPrice) @@ -282,12 +293,10 @@ function calcCustomGasLimit (customGasLimitInHex) { return parseInt(customGasLimitInHex, 16) } -function getTxParams (state, transactionId) { - const { confirmTransaction: { txData }, metamask: { send } } = state - const pendingTransactions = submittedPendingTransactionsSelector(state) - const pendingTransaction = pendingTransactions.find(({ id }) => id === transactionId) - const { txParams: pendingTxParams } = pendingTransaction || {} - return txData.txParams || pendingTxParams || { +function getTxParams (state, selectedTransaction = {}) { + const { metamask: { send } } = state + const { txParams } = selectedTransaction + return txParams || { from: send.from, gas: send.gasLimit || '0x5208', gasPrice: send.gasPrice || getFastPriceEstimateInHexWEI(state, true), diff --git a/ui/app/components/app/gas-customization/gas-modal-page-container/tests/gas-modal-page-container-container.test.js b/ui/app/components/app/gas-customization/gas-modal-page-container/tests/gas-modal-page-container-container.test.js index dbe61d5cf..03d254eee 100644 --- a/ui/app/components/app/gas-customization/gas-modal-page-container/tests/gas-modal-page-container-container.test.js +++ b/ui/app/components/app/gas-customization/gas-modal-page-container/tests/gas-modal-page-container-container.test.js @@ -63,6 +63,9 @@ describe('gas-modal-page-container container', () => { modalState: { props: { hideBasic: true, + txData: { + id: 34, + }, }, }, }, @@ -82,6 +85,14 @@ describe('gas-modal-page-container container', () => { provider: { type: 'mainnet', }, + selectedAddressTxList: [{ + id: 34, + txParams: { + gas: '0x1600000', + gasPrice: '0x3200000', + value: '0x640000000000000', + }, + }], }, gas: { basicEstimates: { @@ -152,6 +163,9 @@ describe('gas-modal-page-container container', () => { maxModeOn: false, selectedToken: null, tokenBalance: '0x0', + transaction: { + id: 34, + }, } const baseMockOwnProps = { transaction: { id: 34 } } const tests = [ @@ -168,7 +182,7 @@ describe('gas-modal-page-container container', () => { mockOwnProps: Object.assign({}, baseMockOwnProps, { transaction: { id: 34, status: 'submitted' }, }), - expectedResult: Object.assign({}, baseExpectedResult, { isSpeedUp: true }), + expectedResult: Object.assign({}, baseExpectedResult, { isSpeedUp: true, transaction: { id: 34 } }), }, { mockState: Object.assign({}, baseMockState, { @@ -317,8 +331,10 @@ describe('gas-modal-page-container container', () => { it('should dispatch a updateGasAndCalculate action with the correct props', () => { mapDispatchToPropsObject.updateConfirmTxGasAndCalculate('ffff', 'aaaa') assert.equal(dispatchSpy.callCount, 3) - assert(confirmTransactionActionSpies.updateGasAndCalculate.calledOnce) - assert.deepEqual(confirmTransactionActionSpies.updateGasAndCalculate.getCall(0).args[0], { gasLimit: 'ffff', gasPrice: 'aaaa' }) + assert(actionSpies.setGasPrice.calledOnce) + assert(actionSpies.setGasLimit.calledOnce) + assert.equal(actionSpies.setGasLimit.getCall(0).args[0], 'ffff') + assert.equal(actionSpies.setGasPrice.getCall(0).args[0], 'aaaa') }) }) @@ -337,6 +353,7 @@ describe('gas-modal-page-container container', () => { }, isConfirm: true, someOtherStateProp: 'baz', + transaction: {}, } dispatchProps = { updateCustomGasPrice: sinon.spy(), -- cgit