Merge pull request #6122 from whymarrh/fix-gas-editing

Fix advanced inline gas editing

7 files changed, 147 insertions, 76 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/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
               ? <AdvancedGasInputs
-                updateCustomGasPrice={newGasPrice => 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..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
@@ -17,12 +17,6 @@ import {
   getDefaultActiveButtonIndex,
 } from '../../../../selectors/custom-gas'
 import {
-  decGWEIToHexWEI,
-  decimalToHex,
-  convertGasPriceForInputs,
-  convertGasLimitForInputs,
-} from '../../../../helpers/conversions.util'
-import {
   showGasButtonGroup,
 } from '../../../../ducks/send.duck'
 import {
@@ -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,16 +72,18 @@ 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(setGasTotal(calcGasTotal(gasLimit, newPrice)))
+      dispatch(setCustomGasPrice(newPrice))
+      if (gasLimit) {
+        dispatch(setGasTotal(calcGasTotal(gasLimit, newPrice)))
+      }
     },
     setGasLimit: (newLimit, gasPrice) => {
-      newLimit = decimalToHex(newLimit)
       dispatch(setGasLimit(newLimit))
-      dispatch(setCustomGasLimit(addHexPrefix(newLimit.toString(16))))
-      dispatch(setGasTotal(calcGasTotal(newLimit, gasPrice)))
+      dispatch(setCustomGasLimit(newLimit))
+      if (gasPrice) {
+        dispatch(setGasTotal(calcGasTotal(newLimit, gasPrice)))
+      }
     },
     showGasButtonGroup: () => dispatch(showGasButtonGroup()),
     resetCustomData: () => dispatch(resetCustomData()),
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)
-}