From 57ead4914f6f9fd1fe47b865559e4908a36f6a8a Mon Sep 17 00:00:00 2001 From: Whymarrh Whitby Date: Fri, 8 Feb 2019 10:20:25 -0330 Subject: [PATCH 1/2] Fix inline advanced gas editing --- .../advanced-gas-inputs.container.js | 28 ++++++++++++++++++- .../confirm-transaction-base.component.js | 18 ++++++------ .../confirm-transaction-base.container.js | 19 ++----------- .../send-gas-row/send-gas-row.container.js | 20 ++++--------- .../tests/send-gas-row-container.test.js | 22 ++++++--------- ui/app/helpers/conversions.util.js | 8 ------ 6 files changed, 51 insertions(+), 64 deletions(-) diff --git a/ui/app/components/gas-customization/advanced-gas-inputs/advanced-gas-inputs.container.js b/ui/app/components/gas-customization/advanced-gas-inputs/advanced-gas-inputs.container.js index 883d11c6d..a71d37b43 100644 --- a/ui/app/components/gas-customization/advanced-gas-inputs/advanced-gas-inputs.container.js +++ b/ui/app/components/gas-customization/advanced-gas-inputs/advanced-gas-inputs.container.js @@ -1,7 +1,20 @@ import { connect } from 'react-redux' import { showModal } from '../../../actions' +import { + decGWEIToHexWEI, + decimalToHex, + hexWEIToDecGWEI, +} from '../../../helpers/conversions.util' import AdvancedGasInputs from './advanced-gas-inputs.component' +function convertGasPriceForInputs (gasPriceInHexWEI) { + return Number(hexWEIToDecGWEI(gasPriceInHexWEI)) +} + +function convertGasLimitForInputs (gasLimitInHexWEI) { + return parseInt(gasLimitInHexWEI, 16) +} + const mapDispatchToProps = dispatch => { return { showGasPriceInfoModal: modalName => dispatch(showModal({ name: 'GAS_PRICE_INFO_MODAL' })), @@ -9,4 +22,17 @@ const mapDispatchToProps = dispatch => { } } -export default connect(null, mapDispatchToProps)(AdvancedGasInputs) +const mergeProps = (stateProps, dispatchProps, ownProps) => { + const {customGasPrice, customGasLimit, updateCustomGasPrice, updateCustomGasLimit} = ownProps + return { + ...stateProps, + ...dispatchProps, + ...ownProps, + customGasPrice: convertGasPriceForInputs(customGasPrice), + customGasLimit: convertGasLimitForInputs(customGasLimit), + updateCustomGasPrice: (price) => updateCustomGasPrice(decGWEIToHexWEI(price)), + updateCustomGasLimit: (limit) => updateCustomGasLimit(decimalToHex(limit)), + } +} + +export default connect(null, mapDispatchToProps, mergeProps)(AdvancedGasInputs) diff --git a/ui/app/components/pages/confirm-transaction-base/confirm-transaction-base.component.js b/ui/app/components/pages/confirm-transaction-base/confirm-transaction-base.component.js index 8d404aaca..d99f03308 100644 --- a/ui/app/components/pages/confirm-transaction-base/confirm-transaction-base.component.js +++ b/ui/app/components/pages/confirm-transaction-base/confirm-transaction-base.component.js @@ -58,6 +58,8 @@ export default class ConfirmTransactionBase extends Component { txData: PropTypes.object, unapprovedTxCount: PropTypes.number, currentNetworkUnapprovedTxs: PropTypes.object, + updateGasAndCalculate: PropTypes.func, + customGas: PropTypes.object, // Component props action: PropTypes.string, contentComponent: PropTypes.node, @@ -83,10 +85,7 @@ export default class ConfirmTransactionBase extends Component { valid: PropTypes.bool, warning: PropTypes.string, advancedInlineGasShown: PropTypes.bool, - gasPrice: PropTypes.number, - gasLimit: PropTypes.number, insufficientBalance: PropTypes.bool, - convertThenUpdateGasAndCalculate: PropTypes.func, } state = { @@ -172,10 +171,9 @@ export default class ConfirmTransactionBase extends Component { hexTransactionTotal, hideDetails, advancedInlineGasShown, - gasPrice, - gasLimit, + customGas, insufficientBalance, - convertThenUpdateGasAndCalculate, + updateGasAndCalculate, } = this.props if (hideDetails) { @@ -195,10 +193,10 @@ export default class ConfirmTransactionBase extends Component { /> {advancedInlineGasShown ? convertThenUpdateGasAndCalculate({ gasPrice: newGasPrice, gasLimit })} - updateCustomGasLimit={newGasLimit => convertThenUpdateGasAndCalculate({ gasLimit: newGasLimit, gasPrice })} - customGasPrice={gasPrice} - customGasLimit={gasLimit} + updateCustomGasPrice={newGasPrice => updateGasAndCalculate({ ...customGas, gasPrice: newGasPrice })} + updateCustomGasLimit={newGasLimit => updateGasAndCalculate({ ...customGas, gasLimit: newGasLimit })} + customGasPrice={customGas.gasPrice} + customGasLimit={customGas.gasLimit} insufficientBalance={insufficientBalance} customPriceIsSafe={true} isSpeedUp={false} diff --git a/ui/app/components/pages/confirm-transaction-base/confirm-transaction-base.container.js b/ui/app/components/pages/confirm-transaction-base/confirm-transaction-base.container.js index 98cde4b03..2ecbe4a64 100644 --- a/ui/app/components/pages/confirm-transaction-base/confirm-transaction-base.container.js +++ b/ui/app/components/pages/confirm-transaction-base/confirm-transaction-base.container.js @@ -14,12 +14,6 @@ import { GAS_LIMIT_TOO_LOW_ERROR_KEY, } from '../../../constants/error-keys' import { getHexGasTotal } from '../../../helpers/confirm-transaction/util' -import { - convertGasPriceForInputs, - convertGasLimitForInputs, - decimalToHex, - decGWEIToHexWEI, -} from '../../../helpers/conversions.util' import { isBalanceSufficient, calcGasTotal } from '../../send/send.utils' import { conversionGreaterThan } from '../../../conversion-util' import { MIN_GAS_LIMIT_DEC } from '../../send/send.constants' @@ -132,12 +126,10 @@ const mapStateToProps = (state, props) => { unapprovedTxCount, currentNetworkUnapprovedTxs, customGas: { - gasLimit: customGasLimit || gasPrice, - gasPrice: customGasPrice || gasLimit, + gasLimit: customGasLimit || gasLimit, + gasPrice: customGasPrice || gasPrice, }, advancedInlineGasShown: getAdvancedInlineGasShown(state), - gasPrice: convertGasPriceForInputs(gasPrice), - gasLimit: convertGasLimitForInputs(gasLimit), insufficientBalance, } } @@ -155,12 +147,6 @@ const mapDispatchToProps = dispatch => { updateGasAndCalculate: ({ gasLimit, gasPrice }) => { return dispatch(updateGasAndCalculate({ gasLimit, gasPrice })) }, - convertThenUpdateGasAndCalculate: ({ gasLimit, gasPrice }) => { - return dispatch(updateGasAndCalculate({ - gasLimit: decimalToHex(gasLimit), - gasPrice: decGWEIToHexWEI(gasPrice), - })) - }, showRejectTransactionsConfirmationModal: ({ onSubmit, unapprovedTxCount }) => { return dispatch(showModal({ name: 'REJECT_TRANSACTIONS', onSubmit, unapprovedTxCount })) }, @@ -235,6 +221,7 @@ const mergeProps = (stateProps, dispatchProps, ownProps) => { validate: validateEditGas, }), cancelAllTransactions: () => dispatchCancelAllTransactions(valuesFor(unapprovedTxs)), + updateGasAndCalculate, } } diff --git a/ui/app/components/send/send-content/send-gas-row/send-gas-row.container.js b/ui/app/components/send/send-content/send-gas-row/send-gas-row.container.js index 96a6de424..a3bc73256 100644 --- a/ui/app/components/send/send-content/send-gas-row/send-gas-row.container.js +++ b/ui/app/components/send/send-content/send-gas-row/send-gas-row.container.js @@ -16,12 +16,6 @@ import { getRenderableEstimateDataForSmallButtonsFromGWEI, getDefaultActiveButtonIndex, } from '../../../../selectors/custom-gas' -import { - decGWEIToHexWEI, - decimalToHex, - convertGasPriceForInputs, - convertGasLimitForInputs, -} from '../../../../helpers/conversions.util' import { showGasButtonGroup, } from '../../../../ducks/send.duck' @@ -33,7 +27,6 @@ import { import { getGasLoadingError, gasFeeIsInError, getGasButtonGroupShown } from './send-gas-row.selectors.js' import { showModal, setGasPrice, setGasLimit, setGasTotal } from '../../../../actions' import { getAdvancedInlineGasShown, getCurrentEthBalance } from '../../../../selectors' -import { addHexPrefix } from 'ethereumjs-util' import SendGasRow from './send-gas-row.component' export default connect(mapStateToProps, mapDispatchToProps, mergeProps)(SendGasRow) @@ -41,9 +34,8 @@ export default connect(mapStateToProps, mapDispatchToProps, mergeProps)(SendGasR function mapStateToProps (state) { const gasButtonInfo = getRenderableEstimateDataForSmallButtonsFromGWEI(state) const gasPrice = getGasPrice(state) + const gasLimit = getGasLimit(state) const activeButtonIndex = getDefaultActiveButtonIndex(gasButtonInfo, gasPrice) - const renderableGasPrice = convertGasPriceForInputs(gasPrice) - const renderableGasLimit = convertGasLimitForInputs(getGasLimit(state)) const gasTotal = getGasTotal(state) const conversionRate = getConversionRate(state) @@ -70,8 +62,8 @@ function mapStateToProps (state) { }, gasButtonGroupShown: getGasButtonGroupShown(state), advancedInlineGasShown: getAdvancedInlineGasShown(state), - gasPrice: renderableGasPrice, - gasLimit: renderableGasLimit, + gasPrice, + gasLimit, insufficientBalance, } } @@ -80,15 +72,13 @@ function mapDispatchToProps (dispatch) { return { showCustomizeGasModal: () => dispatch(showModal({ name: 'CUSTOMIZE_GAS', hideBasic: true })), setGasPrice: (newPrice, gasLimit) => { - newPrice = decGWEIToHexWEI(newPrice) dispatch(setGasPrice(newPrice)) - dispatch(setCustomGasPrice(addHexPrefix(newPrice))) + dispatch(setCustomGasPrice(newPrice)) dispatch(setGasTotal(calcGasTotal(gasLimit, newPrice))) }, setGasLimit: (newLimit, gasPrice) => { - newLimit = decimalToHex(newLimit) dispatch(setGasLimit(newLimit)) - dispatch(setCustomGasLimit(addHexPrefix(newLimit.toString(16)))) + dispatch(setCustomGasLimit(newLimit)) dispatch(setGasTotal(calcGasTotal(newLimit, gasPrice))) }, showGasButtonGroup: () => dispatch(showGasButtonGroup()), diff --git a/ui/app/components/send/send-content/send-gas-row/tests/send-gas-row-container.test.js b/ui/app/components/send/send-content/send-gas-row/tests/send-gas-row-container.test.js index 0d5dc4117..723c406f7 100644 --- a/ui/app/components/send/send-content/send-gas-row/tests/send-gas-row-container.test.js +++ b/ui/app/components/send/send-content/send-gas-row/tests/send-gas-row-container.test.js @@ -66,12 +66,6 @@ proxyquire('../send-gas-row.container.js', { }, '../../../../ducks/send.duck': sendDuckSpies, '../../../../ducks/gas.duck': gasDuckSpies, - '../../../../helpers/conversions.util': { - convertGasPriceForInputs: str => str + '*', - convertGasLimitForInputs: str => str + '**', - decGWEIToHexWEI: str => '0x' + str + '000', - decimalToHex: str => '0x' + str, - }, }) describe('send-gas-row container', () => { @@ -93,8 +87,8 @@ describe('send-gas-row container', () => { }, gasButtonGroupShown: `mockGetGasButtonGroupShown:mockState`, advancedInlineGasShown: 'mockAdvancedInlineGasShown:mockState', - gasLimit: 'mockGasLimit:mockState**', - gasPrice: 'mockGasPrice:mockState*', + gasLimit: 'mockGasLimit:mockState', + gasPrice: 'mockGasPrice:mockState', insufficientBalance: false, }) }) @@ -127,10 +121,10 @@ describe('send-gas-row container', () => { mapDispatchToPropsObject.setGasPrice('mockNewPrice', 'mockLimit') assert(dispatchSpy.calledThrice) assert(actionSpies.setGasPrice.calledOnce) - assert.equal(actionSpies.setGasPrice.getCall(0).args[0], '0xmockNewPrice000') - assert.equal(gasDuckSpies.setCustomGasPrice.getCall(0).args[0], '0xmockNewPrice000') + assert.equal(actionSpies.setGasPrice.getCall(0).args[0], 'mockNewPrice') + assert.equal(gasDuckSpies.setCustomGasPrice.getCall(0).args[0], 'mockNewPrice') assert(actionSpies.setGasTotal.calledOnce) - assert.equal(actionSpies.setGasTotal.getCall(0).args[0], 'mockLimit0xmockNewPrice000') + assert.equal(actionSpies.setGasTotal.getCall(0).args[0], 'mockLimitmockNewPrice') }) }) @@ -139,10 +133,10 @@ describe('send-gas-row container', () => { mapDispatchToPropsObject.setGasLimit('mockNewLimit', 'mockPrice') assert(dispatchSpy.calledThrice) assert(actionSpies.setGasLimit.calledOnce) - assert.equal(actionSpies.setGasLimit.getCall(0).args[0], '0xmockNewLimit') - assert.equal(gasDuckSpies.setCustomGasLimit.getCall(0).args[0], '0xmockNewLimit') + assert.equal(actionSpies.setGasLimit.getCall(0).args[0], 'mockNewLimit') + assert.equal(gasDuckSpies.setCustomGasLimit.getCall(0).args[0], 'mockNewLimit') assert(actionSpies.setGasTotal.calledOnce) - assert.equal(actionSpies.setGasTotal.getCall(0).args[0], '0xmockNewLimitmockPrice') + assert.equal(actionSpies.setGasTotal.getCall(0).args[0], 'mockNewLimitmockPrice') }) }) diff --git a/ui/app/helpers/conversions.util.js b/ui/app/helpers/conversions.util.js index d2aaeca33..065d67e8e 100644 --- a/ui/app/helpers/conversions.util.js +++ b/ui/app/helpers/conversions.util.js @@ -120,11 +120,3 @@ export function hexWEIToDecGWEI (decGWEI) { toDenomination: 'GWEI', }) } - -export function convertGasPriceForInputs (gasPriceInHexWEI) { - return Number(hexWEIToDecGWEI(gasPriceInHexWEI)) -} - -export function convertGasLimitForInputs (gasLimitInHexWEI) { - return parseInt(gasLimitInHexWEI, 16) -} From 0972e23dcd9c15abe9a3229f9514b3b2f1ccd673 Mon Sep 17 00:00:00 2001 From: Whymarrh Whitby Date: Fri, 8 Feb 2019 10:52:19 -0330 Subject: [PATCH 2/2] Add e2e tests adjusting gas before sending --- test/e2e/beta/metamask-beta-ui.spec.js | 100 ++++++++++++++++-- .../send-gas-row/send-gas-row.container.js | 8 +- 2 files changed, 96 insertions(+), 12 deletions(-) diff --git a/test/e2e/beta/metamask-beta-ui.spec.js b/test/e2e/beta/metamask-beta-ui.spec.js index 942651fc8..a824763e0 100644 --- a/test/e2e/beta/metamask-beta-ui.spec.js +++ b/test/e2e/beta/metamask-beta-ui.spec.js @@ -352,7 +352,87 @@ describe('MetaMask', function () { }) }) - describe('Send ETH from inside MetaMask', () => { + describe('Send ETH from inside MetaMask using default gas', () => { + it('starts a send transaction', async function () { + const sendButton = await findElement(driver, By.xpath(`//button[contains(text(), 'Send')]`)) + await sendButton.click() + await delay(regularDelayMs) + + const inputAddress = await findElement(driver, By.css('input[placeholder="Recipient Address"]')) + const inputAmount = await findElement(driver, By.css('.unit-input__input')) + await inputAddress.sendKeys('0x2f318C334780961FB129D2a6c30D0763d9a5C970') + await inputAmount.sendKeys('1') + + const inputValue = await inputAmount.getAttribute('value') + assert.equal(inputValue, '1') + await delay(regularDelayMs) + + // Continue to next screen + const nextScreen = await findElement(driver, By.xpath(`//button[contains(text(), 'Next')]`)) + await nextScreen.click() + await delay(regularDelayMs) + }) + + it('confirms the transaction', async function () { + const confirmButton = await findElement(driver, By.xpath(`//button[contains(text(), 'Confirm')]`)) + await confirmButton.click() + await delay(largeDelayMs) + }) + + it('finds the transaction in the transactions list', async function () { + const transactions = await findElements(driver, By.css('.transaction-list-item')) + assert.equal(transactions.length, 1) + + if (process.env.SELENIUM_BROWSER !== 'firefox') { + const txValues = await findElement(driver, By.css('.transaction-list-item__amount--primary')) + await driver.wait(until.elementTextMatches(txValues, /-1\s*ETH/), 10000) + } + }) + }) + + describe('Send ETH from inside MetaMask using fast gas option', () => { + it('starts a send transaction', async function () { + const sendButton = await findElement(driver, By.xpath(`//button[contains(text(), 'Send')]`)) + await sendButton.click() + await delay(regularDelayMs) + + const inputAddress = await findElement(driver, By.css('input[placeholder="Recipient Address"]')) + const inputAmount = await findElement(driver, By.css('.unit-input__input')) + await inputAddress.sendKeys('0x2f318C334780961FB129D2a6c30D0763d9a5C970') + await inputAmount.sendKeys('1') + + const inputValue = await inputAmount.getAttribute('value') + assert.equal(inputValue, '1') + + // Set the gas price + const fastGas = await findElement(driver, By.xpath(`//button/div/div[contains(text(), "Fast")]`)) + await fastGas.click() + await delay(regularDelayMs) + + // Continue to next screen + const nextScreen = await findElement(driver, By.xpath(`//button[contains(text(), 'Next')]`)) + await nextScreen.click() + await delay(regularDelayMs) + }) + + it('confirms the transaction', async function () { + const confirmButton = await findElement(driver, By.xpath(`//button[contains(text(), 'Confirm')]`)) + await confirmButton.click() + await delay(largeDelayMs) + }) + + it('finds the transaction in the transactions list', async function () { + const transactions = await findElements(driver, By.css('.transaction-list-item')) + assert.equal(transactions.length, 2) + + if (process.env.SELENIUM_BROWSER !== 'firefox') { + const txValues = await findElement(driver, By.css('.transaction-list-item__amount--primary')) + await driver.wait(until.elementTextMatches(txValues, /-1\s*ETH/), 10000) + } + }) + }) + + describe('Send ETH from inside MetaMask using advanced gas modal', () => { it('starts a send transaction', async function () { const sendButton = await findElement(driver, By.xpath(`//button[contains(text(), 'Send')]`)) await sendButton.click() @@ -391,7 +471,7 @@ describe('MetaMask', function () { it('finds the transaction in the transactions list', async function () { const transactions = await findElements(driver, By.css('.transaction-list-item')) - assert.equal(transactions.length, 1) + assert.equal(transactions.length, 3) if (process.env.SELENIUM_BROWSER !== 'firefox') { const txValues = await findElement(driver, By.css('.transaction-list-item__amount--primary')) @@ -447,7 +527,7 @@ describe('MetaMask', function () { it('finds the transaction in the transactions list', async function () { const transactions = await findElements(driver, By.css('.transaction-list-item')) - assert.equal(transactions.length, 2) + assert.equal(transactions.length, 4) const txValues = await findElement(driver, By.css('.transaction-list-item__amount--primary')) await driver.wait(until.elementTextMatches(txValues, /-3\s*ETH/), 10000) @@ -487,7 +567,7 @@ describe('MetaMask', function () { }) it('navigates the transactions', async () => { - let navigateTxButtons = await findElements(driver, By.css('.confirm-page-container-navigation__arrow')) + let navigateTxButtons = await findElements(driver, By.css('.confirm-page-container-navigation__arrow'), 20000) assert.equal(navigateTxButtons.length, 4, 'navigation button present') await navigateTxButtons[2].click() @@ -586,7 +666,7 @@ describe('MetaMask', function () { await delay(largeDelayMs * 2) const confirmedTxes = await findElements(driver, By.css('.transaction-list__completed-transactions .transaction-list-item')) - assert.equal(confirmedTxes.length, 3, '3 transactions present') + assert.equal(confirmedTxes.length, 5, '5 transactions present') }) }) @@ -637,7 +717,7 @@ describe('MetaMask', function () { await driver.wait(async () => { const confirmedTxes = await findElements(driver, By.css('.transaction-list__completed-transactions .transaction-list-item')) - return confirmedTxes.length === 4 + return confirmedTxes.length === 6 }, 10000) const txAction = await findElements(driver, By.css('.transaction-list-item__action')) @@ -697,7 +777,7 @@ describe('MetaMask', function () { await driver.wait(async () => { const confirmedTxes = await findElements(driver, By.css('.transaction-list__completed-transactions .transaction-list-item')) - return confirmedTxes.length === 5 + return confirmedTxes.length === 7 }, 10000) const txValues = await findElements(driver, By.css('.transaction-list-item__amount--primary')) @@ -729,7 +809,7 @@ describe('MetaMask', function () { await driver.wait(async () => { const confirmedTxes = await findElements(driver, By.css('.transaction-list__completed-transactions .transaction-list-item')) - return confirmedTxes.length === 6 + return confirmedTxes.length === 8 }, 10000) const txValues = await findElement(driver, By.css('.transaction-list-item__amount--primary')) @@ -743,9 +823,9 @@ describe('MetaMask', function () { const balance = await findElement(driver, By.css('.transaction-view-balance__primary-balance')) await delay(regularDelayMs) if (process.env.SELENIUM_BROWSER !== 'firefox') { - await driver.wait(until.elementTextMatches(balance, /^89.*\s*ETH.*$/), 10000) + await driver.wait(until.elementTextMatches(balance, /^87.*\s*ETH.*$/), 10000) const tokenAmount = await balance.getText() - assert.ok(/^89.*\s*ETH.*$/.test(tokenAmount)) + assert.ok(/^87.*\s*ETH.*$/.test(tokenAmount)) await delay(regularDelayMs) } }) diff --git a/ui/app/components/send/send-content/send-gas-row/send-gas-row.container.js b/ui/app/components/send/send-content/send-gas-row/send-gas-row.container.js index a3bc73256..50cb47178 100644 --- a/ui/app/components/send/send-content/send-gas-row/send-gas-row.container.js +++ b/ui/app/components/send/send-content/send-gas-row/send-gas-row.container.js @@ -74,12 +74,16 @@ function mapDispatchToProps (dispatch) { setGasPrice: (newPrice, gasLimit) => { dispatch(setGasPrice(newPrice)) dispatch(setCustomGasPrice(newPrice)) - dispatch(setGasTotal(calcGasTotal(gasLimit, newPrice))) + if (gasLimit) { + dispatch(setGasTotal(calcGasTotal(gasLimit, newPrice))) + } }, setGasLimit: (newLimit, gasPrice) => { dispatch(setGasLimit(newLimit)) dispatch(setCustomGasLimit(newLimit)) - dispatch(setGasTotal(calcGasTotal(newLimit, gasPrice))) + if (gasPrice) { + dispatch(setGasTotal(calcGasTotal(newLimit, gasPrice))) + } }, showGasButtonGroup: () => dispatch(showGasButtonGroup()), resetCustomData: () => dispatch(resetCustomData()),