Merge pull request #5089 from MetaMask/i5085-ethcode

Use eth_getCode to sort transaction action type

5 files changed, 99 insertions, 37 deletions

diff --git a/ui/app/components/pages/confirm-transaction-switch/confirm-transaction-switch.component.js b/ui/app/components/pages/confirm-transaction-switch/confirm-transaction-switch.component.js
index 0280f73c6..d494977cd 100644
--- a/ui/app/components/pages/confirm-transaction-switch/confirm-transaction-switch.component.js
+++ b/ui/app/components/pages/confirm-transaction-switch/confirm-transaction-switch.component.js
@@ -23,14 +23,16 @@ export default class ConfirmTransactionSwitch extends Component {
   static propTypes = {
     txData: PropTypes.object,
     methodData: PropTypes.object,
-    fetchingMethodData: PropTypes.bool,
+    fetchingData: PropTypes.bool,
+    isEtherTransaction: PropTypes.bool,
   }
 
   redirectToTransaction () {
     const {
       txData,
       methodData: { name },
-      fetchingMethodData,
+      fetchingData,
+      isEtherTransaction,
     } = this.props
     const { id, txParams: { data } = {} } = txData
 
@@ -39,10 +41,15 @@ export default class ConfirmTransactionSwitch extends Component {
       return <Redirect to={{ pathname }} />
     }
 
-    if (fetchingMethodData) {
+    if (fetchingData) {
       return <Loading />
     }
 
+    if (isEtherTransaction) {
+      const pathname = `${CONFIRM_TRANSACTION_ROUTE}/${id}${CONFIRM_SEND_ETHER_PATH}`
+      return <Redirect to={{ pathname }} />
+    }
+
     if (data) {
       const methodName = name && name.toLowerCase()
 
diff --git a/ui/app/components/pages/confirm-transaction-switch/confirm-transaction-switch.container.js b/ui/app/components/pages/confirm-transaction-switch/confirm-transaction-switch.container.js
index 3d7fc78cc..7f2c36af2 100644
--- a/ui/app/components/pages/confirm-transaction-switch/confirm-transaction-switch.container.js
+++ b/ui/app/components/pages/confirm-transaction-switch/confirm-transaction-switch.container.js
@@ -6,14 +6,16 @@ const mapStateToProps = state => {
     confirmTransaction: {
       txData,
       methodData,
-      fetchingMethodData,
+      fetchingData,
+      toSmartContract,
     },
   } = state
 
   return {
     txData,
     methodData,
-    fetchingMethodData,
+    fetchingData,
+    isEtherTransaction: !toSmartContract,
   }
 }
 
diff --git a/ui/app/ducks/confirm-transaction.duck.js b/ui/app/ducks/confirm-transaction.duck.js
index 1885e12d1..f17933ddd 100644
--- a/ui/app/ducks/confirm-transaction.duck.js
+++ b/ui/app/ducks/confirm-transaction.duck.js
@@ -14,6 +14,7 @@ import {
   addEth,
   increaseLastGasPrice,
   hexGreaterThan,
+  isSmartContractAddress,
 } from '../helpers/confirm-transaction/util'
 
 import { getSymbolAndDecimals } from '../token-util'
@@ -35,8 +36,9 @@ const UPDATE_TRANSACTION_TOTALS = createActionType('UPDATE_TRANSACTION_TOTALS')
 const UPDATE_HEX_GAS_TOTAL = createActionType('UPDATE_HEX_GAS_TOTAL')
 const UPDATE_TOKEN_PROPS = createActionType('UPDATE_TOKEN_PROPS')
 const UPDATE_NONCE = createActionType('UPDATE_NONCE')
-const FETCH_METHOD_DATA_START = createActionType('FETCH_METHOD_DATA_START')
-const FETCH_METHOD_DATA_END = createActionType('FETCH_METHOD_DATA_END')
+const UPDATE_TO_SMART_CONTRACT = createActionType('UPDATE_TO_SMART_CONTRACT')
+const FETCH_DATA_START = createActionType('FETCH_DATA_START')
+const FETCH_DATA_END = createActionType('FETCH_DATA_END')
 
 // Initial state
 const initState = {
@@ -55,7 +57,8 @@ const initState = {
   ethTransactionTotal: '',
   hexGasTotal: '',
   nonce: '',
-  fetchingMethodData: false,
+  toSmartContract: false,
+  fetchingData: false,
 }
 
 // Reducer
@@ -138,15 +141,20 @@ export default function reducer ({ confirmTransaction: confirmState = initState
         ...confirmState,
         nonce: action.payload,
       }
-    case FETCH_METHOD_DATA_START:
+    case UPDATE_TO_SMART_CONTRACT:
       return {
         ...confirmState,
-        fetchingMethodData: true,
+        toSmartContract: action.payload,
       }
-    case FETCH_METHOD_DATA_END:
+    case FETCH_DATA_START:
       return {
         ...confirmState,
-        fetchingMethodData: false,
+        fetchingData: true,
+      }
+    case FETCH_DATA_END:
+      return {
+        ...confirmState,
+        fetchingData: false,
       }
     case CLEAR_CONFIRM_TRANSACTION:
       return initState
@@ -237,9 +245,16 @@ export function updateNonce (nonce) {
   }
 }
 
-export function setFetchingMethodData (isFetching) {
+export function updateToSmartContract (toSmartContract) {
+  return {
+    type: UPDATE_TO_SMART_CONTRACT,
+    payload: toSmartContract,
+  }
+}
+
+export function setFetchingData (isFetching) {
   return {
-    type: isFetching ? FETCH_METHOD_DATA_START : FETCH_METHOD_DATA_END,
+    type: isFetching ? FETCH_DATA_START : FETCH_DATA_END,
   }
 }
 
@@ -338,19 +353,22 @@ export function setTransactionToConfirm (transactionId) {
       dispatch(updateTxDataAndCalculate(txData))
 
       const { txParams } = transaction
+      const { to } = txParams
 
       if (txParams.data) {
         const { tokens: existingTokens } = state
         const { data, to: tokenAddress } = txParams
 
         try {
-          dispatch(setFetchingMethodData(true))
+          dispatch(setFetchingData(true))
           const methodData = await getMethodData(data)
           dispatch(updateMethodData(methodData))
-          dispatch(setFetchingMethodData(false))
+          const toSmartContract = await isSmartContractAddress(to)
+          dispatch(updateToSmartContract(toSmartContract))
+          dispatch(setFetchingData(false))
         } catch (error) {
           dispatch(updateMethodData({}))
-          dispatch(setFetchingMethodData(false))
+          dispatch(setFetchingData(false))
         }
 
         const tokenData = getTokenData(data)
diff --git a/ui/app/ducks/tests/confirm-transaction.duck.test.js b/ui/app/ducks/tests/confirm-transaction.duck.test.js
index 111674e33..1bab0add0 100644
--- a/ui/app/ducks/tests/confirm-transaction.duck.test.js
+++ b/ui/app/ducks/tests/confirm-transaction.duck.test.js
@@ -1,6 +1,7 @@
 import assert from 'assert'
 import configureMockStore from 'redux-mock-store'
 import thunk from 'redux-thunk'
+import sinon from 'sinon'
 
 import ConfirmTransactionReducer, * as actions from '../confirm-transaction.duck.js'
 
@@ -20,7 +21,8 @@ const initialState = {
   ethTransactionTotal: '',
   hexGasTotal: '',
   nonce: '',
-  fetchingMethodData: false,
+  toSmartContract: false,
+  fetchingData: false,
 }
 
 const UPDATE_TX_DATA = 'metamask/confirm-transaction/UPDATE_TX_DATA'
@@ -35,8 +37,9 @@ const UPDATE_TRANSACTION_TOTALS = 'metamask/confirm-transaction/UPDATE_TRANSACTI
 const UPDATE_HEX_GAS_TOTAL = 'metamask/confirm-transaction/UPDATE_HEX_GAS_TOTAL'
 const UPDATE_TOKEN_PROPS = 'metamask/confirm-transaction/UPDATE_TOKEN_PROPS'
 const UPDATE_NONCE = 'metamask/confirm-transaction/UPDATE_NONCE'
-const FETCH_METHOD_DATA_START = 'metamask/confirm-transaction/FETCH_METHOD_DATA_START'
-const FETCH_METHOD_DATA_END = 'metamask/confirm-transaction/FETCH_METHOD_DATA_END'
+const UPDATE_TO_SMART_CONTRACT = 'metamask/confirm-transaction/UPDATE_TO_SMART_CONTRACT'
+const FETCH_DATA_START = 'metamask/confirm-transaction/FETCH_DATA_START'
+const FETCH_DATA_END = 'metamask/confirm-transaction/FETCH_DATA_END'
 const CLEAR_CONFIRM_TRANSACTION = 'metamask/confirm-transaction/CLEAR_CONFIRM_TRANSACTION'
 
 describe('Confirm Transaction Duck', () => {
@@ -64,7 +67,8 @@ describe('Confirm Transaction Duck', () => {
         ethTransactionTotal: '469.27',
         hexGasTotal: '0x1319718a5000',
         nonce: '0x0',
-        fetchingMethodData: false,
+        toSmartContract: false,
+        fetchingData: false,
       },
     }
 
@@ -271,30 +275,43 @@ describe('Confirm Transaction Duck', () => {
       )
     })
 
-    it('should set fetchingMethodData to true when receiving a FETCH_METHOD_DATA_START action', () => {
+    it('should update nonce when receiving an UPDATE_TO_SMART_CONTRACT action', () => {
       assert.deepEqual(
         ConfirmTransactionReducer(mockState, {
-          type: FETCH_METHOD_DATA_START,
+          type: UPDATE_TO_SMART_CONTRACT,
+          payload: true,
         }),
         {
           ...mockState.confirmTransaction,
-          fetchingMethodData: true,
+          toSmartContract: true,
         }
       )
     })
 
-    it('should set fetchingMethodData to false when receiving a FETCH_METHOD_DATA_END action', () => {
+    it('should set fetchingData to true when receiving a FETCH_DATA_START action', () => {
       assert.deepEqual(
-        ConfirmTransactionReducer({ confirmTransaction: { fetchingMethodData: true } }, {
-          type: FETCH_METHOD_DATA_END,
+        ConfirmTransactionReducer(mockState, {
+          type: FETCH_DATA_START,
         }),
         {
-          fetchingMethodData: false,
+          ...mockState.confirmTransaction,
+          fetchingData: true,
         }
       )
     })
 
-    it('should clear confirmTransaction when receiving a FETCH_METHOD_DATA_END action', () => {
+    it('should set fetchingData to false when receiving a FETCH_DATA_END action', () => {
+      assert.deepEqual(
+        ConfirmTransactionReducer({ confirmTransaction: { fetchingData: true } }, {
+          type: FETCH_DATA_END,
+        }),
+        {
+          fetchingData: false,
+        }
+      )
+    })
+
+    it('should clear confirmTransaction when receiving a FETCH_DATA_END action', () => {
       assert.deepEqual(
         ConfirmTransactionReducer(mockState, {
           type: CLEAR_CONFIRM_TRANSACTION,
@@ -460,24 +477,24 @@ describe('Confirm Transaction Duck', () => {
       )
     })
 
-    it('should create an action to set fetchingMethodData to true', () => {
+    it('should create an action to set fetchingData to true', () => {
       const expectedAction = {
-        type: FETCH_METHOD_DATA_START,
+        type: FETCH_DATA_START,
       }
 
       assert.deepEqual(
-        actions.setFetchingMethodData(true),
+        actions.setFetchingData(true),
         expectedAction
       )
     })
 
-    it('should create an action to set fetchingMethodData to false', () => {
+    it('should create an action to set fetchingData to false', () => {
       const expectedAction = {
-        type: FETCH_METHOD_DATA_END,
+        type: FETCH_DATA_END,
       }
 
       assert.deepEqual(
-        actions.setFetchingMethodData(false),
+        actions.setFetchingData(false),
         expectedAction
       )
     })
@@ -495,6 +512,18 @@ describe('Confirm Transaction Duck', () => {
   })
 
   describe('Thunk actions', done => {
+    beforeEach(() => {
+      global.eth = {
+        getCode: sinon.stub().callsFake(
+          address => Promise.resolve(address && address.match(/isContract/) ? 'not-0x' : '0x')
+        ),
+      }
+    })
+
+    afterEach(() => {
+      global.eth.getCode.resetHistory()
+    })
+
     it('updates txData and gas on an existing transaction in confirmTransaction', () => {
       const mockState = {
         metamask: {
@@ -505,7 +534,7 @@ describe('Confirm Transaction Duck', () => {
           ethTransactionAmount: '1',
           ethTransactionFee: '0.000021',
           ethTransactionTotal: '1.000021',
-          fetchingMethodData: false,
+          fetchingData: false,
           fiatTransactionAmount: '469.26',
           fiatTransactionFee: '0.01',
           fiatTransactionTotal: '469.27',
@@ -581,7 +610,7 @@ describe('Confirm Transaction Duck', () => {
           ethTransactionAmount: '1',
           ethTransactionFee: '0.000021',
           ethTransactionTotal: '1.000021',
-          fetchingMethodData: false,
+          fetchingData: false,
           fiatTransactionAmount: '469.26',
           fiatTransactionFee: '0.01',
           fiatTransactionTotal: '469.27',
@@ -667,6 +696,7 @@ describe('Confirm Transaction Duck', () => {
         .then(() => {
           const storeActions = store.getActions()
           assert.equal(storeActions.length, expectedActions.length)
+
           storeActions.forEach((action, index) => assert.equal(action.type, expectedActions[index]))
           done()
         })
diff --git a/ui/app/helpers/confirm-transaction/util.js b/ui/app/helpers/confirm-transaction/util.js
index 76e80a8ac..3d0cb57e7 100644
--- a/ui/app/helpers/confirm-transaction/util.js
+++ b/ui/app/helpers/confirm-transaction/util.js
@@ -146,3 +146,8 @@ export function roundExponential (value) {
   // In JS, numbers with exponentials greater than 20 get displayed as an exponential.
   return bigNumberValue.e > 20 ? Number(bigNumberValue.toPrecision(PRECISION)) : value
 }
+
+export async function isSmartContractAddress (address) {
+  const code = await global.eth.getCode(address)
+  return code && code !== '0x'
+}