Fix calculation of data property for gas estimation on token transfers.

7 files changed, 41 insertions, 47 deletions

diff --git a/ui/app/actions.js b/ui/app/actions.js
index b6cb9c382..465b3d5fe 100644
--- a/ui/app/actions.js
+++ b/ui/app/actions.js
@@ -732,7 +732,6 @@ function setGasTotal (gasTotal) {
 
 function updateGasData ({
   blockGasLimit,
-  data,
   recentBlocks,
   selectedAddress,
   selectedToken,
@@ -746,7 +745,6 @@ function updateGasData ({
       estimateGas({
         estimateGasMethod: background.estimateGas,
         blockGasLimit,
-        data,
         selectedAddress,
         selectedToken,
         to,
diff --git a/ui/app/components/send_/send.component.js b/ui/app/components/send_/send.component.js
index 97c6d1294..516251e22 100644
--- a/ui/app/components/send_/send.component.js
+++ b/ui/app/components/send_/send.component.js
@@ -20,7 +20,6 @@ export default class SendTransactionScreen extends PersistentForm {
     ]),
     blockGasLimit: PropTypes.string,
     conversionRate: PropTypes.number,
-    data: PropTypes.string,
     editingTransactionId: PropTypes.string,
     from: PropTypes.object,
     gasLimit: PropTypes.string,
@@ -43,7 +42,6 @@ export default class SendTransactionScreen extends PersistentForm {
     const {
       amount,
       blockGasLimit,
-      data,
       editingTransactionId,
       gasLimit,
       gasPrice,
@@ -55,7 +53,6 @@ export default class SendTransactionScreen extends PersistentForm {
 
     updateAndSetGasTotal({
       blockGasLimit,
-      data,
       editingTransactionId,
       gasLimit,
       gasPrice,
diff --git a/ui/app/components/send_/send.container.js b/ui/app/components/send_/send.container.js
index 7e241aa2d..1fd96d61f 100644
--- a/ui/app/components/send_/send.container.js
+++ b/ui/app/components/send_/send.container.js
@@ -31,7 +31,6 @@ import {
 } from '../../ducks/send.duck'
 import {
   calcGasTotal,
-  generateTokenTransferData,
 } from './send.utils.js'
 
 module.exports = compose(
@@ -40,15 +39,11 @@ module.exports = compose(
 )(SendEther)
 
 function mapStateToProps (state) {
-  const selectedAddress = getSelectedAddress(state)
-  const selectedToken = getSelectedToken(state)
-
   return {
     amount: getSendAmount(state),
     amountConversionRate: getAmountConversionRate(state),
     blockGasLimit: getBlockGasLimit(state),
     conversionRate: getConversionRate(state),
-    data: generateTokenTransferData(selectedAddress, selectedToken),
     editingTransactionId: getSendEditingTransactionId(state),
     from: getSendFromObject(state),
     gasLimit: getGasLimit(state),
@@ -69,7 +64,6 @@ function mapDispatchToProps (dispatch) {
   return {
     updateAndSetGasTotal: ({
       blockGasLimit,
-      data,
       editingTransactionId,
       gasLimit,
       gasPrice,
@@ -80,7 +74,7 @@ function mapDispatchToProps (dispatch) {
       value,
     }) => {
       !editingTransactionId
-        ? dispatch(updateGasData({ recentBlocks, selectedAddress, selectedToken, data, blockGasLimit, to, value }))
+        ? dispatch(updateGasData({ recentBlocks, selectedAddress, selectedToken, blockGasLimit, to, value }))
         : dispatch(setGasTotal(calcGasTotal(gasLimit, gasPrice)))
     },
     updateSendTokenBalance: ({ selectedToken, tokenContract, address }) => {
diff --git a/ui/app/components/send_/send.utils.js b/ui/app/components/send_/send.utils.js
index e685cc274..855d12303 100644
--- a/ui/app/components/send_/send.utils.js
+++ b/ui/app/components/send_/send.utils.js
@@ -14,6 +14,7 @@ const {
   NEGATIVE_ETH_ERROR,
   ONE_GWEI_IN_WEI_HEX,
   SIMPLE_GAS_COST,
+  TOKEN_TRANSFER_FUNCTION_SIGNATURE,
 } = require('./send.constants')
 const abi = require('ethereumjs-abi')
 const ethUtil = require('ethereumjs-util')
@@ -165,26 +166,23 @@ function doesAmountErrorRequireUpdate ({
   return amountErrorRequiresUpdate
 }
 
-async function estimateGas ({ selectedAddress, selectedToken, data, blockGasLimit, to, value, gasPrice, estimateGasMethod }) {
-  const { symbol } = selectedToken || {}
+async function estimateGas ({ selectedAddress, selectedToken, blockGasLimit, to, value, gasPrice, estimateGasMethod }) {
   const paramsForGasEstimate = { from: selectedAddress, value, gasPrice }
 
-  if (symbol) {
-    Object.assign(paramsForGasEstimate, { value: '0x0' })
+  if (selectedToken) {
+    paramsForGasEstimate.value = '0x0'
+    paramsForGasEstimate.data = generateTokenTransferData({ toAddress: to, amount: value, selectedToken })
   }
 
-  if (data) {
-    Object.assign(paramsForGasEstimate, { data })
-  }
   // if recipient has no code, gas is 21k max:
   const hasRecipient = Boolean(to)
   let code
   if (hasRecipient) code = await global.eth.getCode(to)
-  if (hasRecipient && (!code || code === '0x')) {
+  if (hasRecipient && (!code || code === '0x') && !selectedToken) {
     return SIMPLE_GAS_COST
   }
 
-  paramsForGasEstimate.to = to
+  paramsForGasEstimate.to = selectedToken ? selectedToken.address : to
 
   // if not, fall back to block gasLimit
   paramsForGasEstimate.gas = ethUtil.addHexPrefix(multiplyCurrencies(blockGasLimit, 0.95, {
@@ -212,11 +210,10 @@ async function estimateGas ({ selectedAddress, selectedToken, data, blockGasLimi
   })
 }
 
-function generateTokenTransferData (selectedAddress, selectedToken) {
+function generateTokenTransferData ({ toAddress = '0x0', amount = '0x0', selectedToken }) {
   if (!selectedToken) return
-  console.log(`abi.rawEncode`, abi.rawEncode)
-  return Array.prototype.map.call(
-    abi.rawEncode(['address', 'uint256'], [selectedAddress, '0x0']),
+  return TOKEN_TRANSFER_FUNCTION_SIGNATURE + Array.prototype.map.call(
+    abi.rawEncode(['address', 'uint256'], [toAddress, ethUtil.addHexPrefix(amount)]),
     x => ('00' + x.toString(16)).slice(-2)
   ).join('')
 }
diff --git a/ui/app/components/send_/tests/send-component.test.js b/ui/app/components/send_/tests/send-component.test.js
index 2529d6e5f..4e33d8f63 100644
--- a/ui/app/components/send_/tests/send-component.test.js
+++ b/ui/app/components/send_/tests/send-component.test.js
@@ -34,7 +34,6 @@ describe('Send Component', function () {
       amountConversionRate={'mockAmountConversionRate'}
       blockGasLimit={'mockBlockGasLimit'}
       conversionRate={10}
-      data={'mockData'}
       editingTransactionId={'mockEditingTransactionId'}
       from={ { address: 'mockAddress', balance: 'mockBalance' } }
       gasLimit={'mockGasLimit'}
@@ -210,7 +209,6 @@ describe('Send Component', function () {
         propsMethodSpies.updateAndSetGasTotal.getCall(0).args[0],
         {
           blockGasLimit: 'mockBlockGasLimit',
-          data: 'mockData',
           editingTransactionId: 'mockEditingTransactionId',
           gasLimit: 'mockGasLimit',
           gasPrice: 'mockGasPrice',
diff --git a/ui/app/components/send_/tests/send-container.test.js b/ui/app/components/send_/tests/send-container.test.js
index d077ab4ee..056aad148 100644
--- a/ui/app/components/send_/tests/send-container.test.js
+++ b/ui/app/components/send_/tests/send-container.test.js
@@ -47,7 +47,6 @@ proxyquire('../send.container.js', {
   '../../ducks/send.duck': duckActionSpies,
   './send.utils.js': {
     calcGasTotal: (gasLimit, gasPrice) => gasLimit + gasPrice,
-    generateTokenTransferData: (a, b) => `mockData:${a + b}`,
   },
 })
 
@@ -61,7 +60,6 @@ describe('send container', () => {
         amountConversionRate: 'mockAmountConversionRate:mockState',
         blockGasLimit: 'mockBlockGasLimit:mockState',
         conversionRate: 'mockConversionRate:mockState',
-        data: 'mockData:mockSelectedAddress:mockStatemockSelectedToken:mockState',
         editingTransactionId: 'mockEditingTransactionId:mockState',
         from: 'mockFrom:mockState',
         gasLimit: 'mockGasLimit:mockState',
@@ -92,7 +90,6 @@ describe('send container', () => {
     describe('updateAndSetGasTotal()', () => {
       const mockProps = {
         blockGasLimit: 'mockBlockGasLimit',
-        data: '0x1',
         editingTransactionId: '0x2',
         gasLimit: '0x3',
         gasPrice: '0x4',
@@ -113,14 +110,14 @@ describe('send container', () => {
       })
 
       it('should dispatch an updateGasData action when editingTransactionId is falsy', () => {
-        const { selectedAddress, selectedToken, data, recentBlocks, blockGasLimit, to, value } = mockProps
+        const { selectedAddress, selectedToken, recentBlocks, blockGasLimit, to, value } = mockProps
         mapDispatchToPropsObject.updateAndSetGasTotal(
           Object.assign({}, mockProps, {editingTransactionId: false})
         )
         assert(dispatchSpy.calledOnce)
         assert.deepEqual(
           actionSpies.updateGasData.getCall(0).args[0],
-          { selectedAddress, selectedToken, data, recentBlocks, blockGasLimit, to, value }
+          { selectedAddress, selectedToken, recentBlocks, blockGasLimit, to, value }
         )
       })
     })
diff --git a/ui/app/components/send_/tests/send-utils.test.js b/ui/app/components/send_/tests/send-utils.test.js
index 14125d7a6..b3f6372ef 100644
--- a/ui/app/components/send_/tests/send-utils.test.js
+++ b/ui/app/components/send_/tests/send-utils.test.js
@@ -106,11 +106,23 @@ describe('send utils', () => {
 
   describe('generateTokenTransferData()', () => {
     it('should return undefined if not passed a selected token', () => {
-      assert.equal(generateTokenTransferData('mockAddress', false), undefined)
+      assert.equal(generateTokenTransferData({ toAddress: 'mockAddress', amount: '0xa', selectedToken: false}), undefined)
+    })
+
+    it('should call abi.rawEncode with the correct params', () => {
+      stubs.rawEncode.resetHistory()
+      generateTokenTransferData({ toAddress: 'mockAddress', amount: 'ab', selectedToken: true})
+      assert.deepEqual(
+        stubs.rawEncode.getCall(0).args,
+        [['address', 'uint256'], ['mockAddress', '0xab']]
+      )
     })
 
     it('should return encoded token transfer data', () => {
-      assert.equal(generateTokenTransferData('mockAddress', true), '104c')
+      assert.equal(
+        generateTokenTransferData({ toAddress: 'mockAddress', amount: '0xa', selectedToken: true}),
+        '0xa9059cbb104c'
+      )
     })
   })
 
@@ -276,22 +288,17 @@ describe('send utils', () => {
       assert.equal(result, 'mockToString:16')
     })
 
-    it('should call ethQuery.estimateGas with a value of 0x0 if the passed selectedToken has a symbol', async () => {
-      const result = await estimateGas(Object.assign({ selectedToken: { symbol: true } }, baseMockParams))
+    it('should call ethQuery.estimateGas with a value of 0x0 and the expected data and to if passed a selectedToken', async () => {
+      const result = await estimateGas(Object.assign({ data: 'mockData', selectedToken: { address: 'mockAddress' } }, baseMockParams))
       assert.equal(baseMockParams.estimateGasMethod.callCount, 1)
       assert.deepEqual(
         baseMockParams.estimateGasMethod.getCall(0).args[0],
-        Object.assign({ gasPrice: undefined, value: '0x0' }, baseExpectedCall)
-      )
-      assert.equal(result, 'mockToString:16')
-    })
-
-    it('should call ethQuery.estimateGas with data if data is passed', async () => {
-      const result = await estimateGas(Object.assign({ data: 'mockData' }, baseMockParams))
-      assert.equal(baseMockParams.estimateGasMethod.callCount, 1)
-      assert.deepEqual(
-        baseMockParams.estimateGasMethod.getCall(0).args[0],
-        Object.assign({ gasPrice: undefined, value: undefined, data: 'mockData' }, baseExpectedCall)
+        Object.assign({}, baseExpectedCall, {
+          gasPrice: undefined,
+          value: '0x0',
+          data: '0xa9059cbb104c',
+          to: 'mockAddress',
+        })
       )
       assert.equal(result, 'mockToString:16')
     })
@@ -302,6 +309,12 @@ describe('send utils', () => {
       assert.equal(result, SIMPLE_GAS_COST)
     })
 
+    it(`should not return ${SIMPLE_GAS_COST} if passed a selectedToken`, async () => {
+      assert.equal(baseMockParams.estimateGasMethod.callCount, 0)
+      const result = await estimateGas(Object.assign({}, baseMockParams, { to: '0x123', selectedToken: { address: '' } }))
+      assert.notEqual(result, SIMPLE_GAS_COST)
+    })
+
     it(`should return the adjusted blockGasLimit if it fails with a 'Transaction execution error.'`, async () => {
       const result = await estimateGas(Object.assign({}, baseMockParams, {
         to: 'isContract willFailBecauseOf:Transaction execution error.',