Merge pull request #5567 from MetaMask/HowardBraham-develop

Feature: Warn when attempting to send tx with data to non-contract

8 files changed, 50 insertions, 16 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 312260c65..37d9f26b2 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,7 @@
 
 ## Current Develop Branch
 
+- [#5283](https://github.com/MetaMask/metamask-extension/pull/5283): Fix bug when eth.getCode() called with no contract
 - [#5563](https://github.com/MetaMask/metamask-extension/pull/5563#pullrequestreview-166769174) Feature: improve Hatian Creole translations
 - [#5559](https://github.com/MetaMask/metamask-extension/pull/5559) Localize language names in translation select list
 
diff --git a/app/_locales/en/messages.json b/app/_locales/en/messages.json
index 5423c76dc..26d0422cb 100644
--- a/app/_locales/en/messages.json
+++ b/app/_locales/en/messages.json
@@ -1196,6 +1196,9 @@
   "transactionError": {
     "message": "Transaction Error. Exception thrown in contract code."
   },
+  "transactionErrorNoContract": {
+    "message": "Trying to call a function on a non-contract address."
+  },
   "transactionMemo": {
     "message": "Transaction memo (optional)"
   },
diff --git a/app/scripts/controllers/transactions/tx-gas-utils.js b/app/scripts/controllers/transactions/tx-gas-utils.js
index 3dd45507f..def67c2c3 100644
--- a/app/scripts/controllers/transactions/tx-gas-utils.js
+++ b/app/scripts/controllers/transactions/tx-gas-utils.js
@@ -7,6 +7,8 @@ const {
 const { addHexPrefix } = require('ethereumjs-util')
 const SIMPLE_GAS_COST = '0x5208' // Hex for 21000, cost of a simple send.
 
+import { TRANSACTION_NO_CONTRACT_ERROR_KEY } from '../../../../ui/app/constants/error-keys'
+
 /**
 tx-gas-utils are gas utility methods for Transaction manager
 its passed ethquery
@@ -32,6 +34,7 @@ class TxGasUtil {
     } catch (err) {
       txMeta.simulationFails = {
         reason: err.message,
+        errorKey: err.errorKey,
       }
       return txMeta
     }
@@ -56,24 +59,38 @@ class TxGasUtil {
       return txParams.gas
     }
 
-    // if recipient has no code, gas is 21k max:
     const recipient = txParams.to
     const hasRecipient = Boolean(recipient)
-    let code
-    if (recipient) code = await this.query.getCode(recipient)
 
-    if (hasRecipient && (!code || code === '0x')) {
-      txParams.gas = SIMPLE_GAS_COST
-      txMeta.simpleSend = true // Prevents buffer addition
-      return SIMPLE_GAS_COST
+    // see if we can set the gas based on the recipient
+    if (hasRecipient) {
+      const code = await this.query.getCode(recipient)
+      // For an address with no code, geth will return '0x', and ganache-core v2.2.1 will return '0x0'
+      const codeIsEmpty = !code || code === '0x' || code === '0x0'
+
+      if (codeIsEmpty) {
+        // if there's data in the params, but there's no contract code, it's not a valid transaction
+        if (txParams.data) {
+          const err = new Error('TxGasUtil - Trying to call a function on a non-contract address')
+          // set error key so ui can display localized error message
+          err.errorKey = TRANSACTION_NO_CONTRACT_ERROR_KEY
+          throw err
+        }
+
+        // This is a standard ether simple send, gas requirement is exactly 21k
+        txParams.gas = SIMPLE_GAS_COST
+        // prevents buffer addition
+        txMeta.simpleSend = true
+        return SIMPLE_GAS_COST
+      }
     }
 
-    // if not, fall back to block gasLimit
+    // fallback to block gasLimit
     const blockGasLimitBN = hexToBn(blockGasLimitHex)
     const saferGasLimitBN = BnMultiplyByFraction(blockGasLimitBN, 19, 20)
     txParams.gas = bnToHex(saferGasLimitBN)
 
-    // run tx
+    // estimate tx gas requirements
     return await this.query.estimateGas(txParams)
   }
 
diff --git a/old-ui/app/components/pending-tx.js b/old-ui/app/components/pending-tx.js
index c8132539c..35e81210e 100644
--- a/old-ui/app/components/pending-tx.js
+++ b/old-ui/app/components/pending-tx.js
@@ -1,4 +1,5 @@
 const Component = require('react').Component
+const connect = require('react-redux').connect
 const h = require('react-hyperscript')
 const inherits = require('util').inherits
 const actions = require('../../../ui/app/actions')
@@ -19,7 +20,9 @@ const BNInput = require('./bn-as-decimal-input')
 const MIN_GAS_PRICE_BN = new BN('0')
 const MIN_GAS_LIMIT_BN = new BN('21000')
 
-module.exports = PendingTx
+module.exports = connect()(PendingTx)
+
+
 inherits(PendingTx, Component)
 function PendingTx () {
   Component.call(this)
@@ -445,7 +448,8 @@ PendingTx.prototype.onSubmit = function (event) {
   const txMeta = this.gatherTxMeta()
   const valid = this.checkValidity()
   this.setState({ valid, submitting: true })
-  if (valid && this.verifyGasParams()) {
+  const validGasParams = this.verifyGasParams()
+  if (valid && validGasParams) {
     this.props.sendTransaction(txMeta, event)
   } else {
     this.props.dispatch(actions.displayWarning('Invalid Gas Parameters'))
@@ -488,8 +492,12 @@ PendingTx.prototype.verifyGasParams = function () {
   )
 }
 
-PendingTx.prototype._notZeroOrEmptyString = function (obj) {
-  return obj !== '' && obj !== '0x0'
+PendingTx.prototype._notZeroOrEmptyString = function (value) {
+  // allow undefined values
+  if (value === undefined) return true
+  // Geth will return '0x', and ganache-core v2.2.1 will return '0x0'
+  const valueIsEmpty = !value || value === '0x' || value === '0x0'
+  return !valueIsEmpty
 }
 
 PendingTx.prototype.bnMultiplyByFraction = function (targetBN, numerator, denominator) {
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 c92867afe..7d01aaffb 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
@@ -136,7 +136,7 @@ export default class ConfirmTransactionBase extends Component {
     if (simulationFails) {
       return {
         valid: true,
-        errorKey: TRANSACTION_ERROR_KEY,
+        errorKey: simulationFails.errorKey ? simulationFails.errorKey : TRANSACTION_ERROR_KEY,
       }
     }
 
diff --git a/ui/app/components/send/send.utils.js b/ui/app/components/send/send.utils.js
index a18a9e4b3..eb1667c63 100644
--- a/ui/app/components/send/send.utils.js
+++ b/ui/app/components/send/send.utils.js
@@ -215,7 +215,9 @@ async function estimateGas ({
   // if recipient has no code, gas is 21k max:
   if (!selectedToken && !data) {
     const code = Boolean(to) && await global.eth.getCode(to)
-    if (!code || code === '0x') {
+    // Geth will return '0x', and ganache-core v2.2.1 will return '0x0'
+    const codeIsEmpty = !code || code === '0x' || code === '0x0'
+    if (codeIsEmpty) {
       return SIMPLE_GAS_COST
     }
   } else if (selectedToken && !to) {
diff --git a/ui/app/constants/error-keys.js b/ui/app/constants/error-keys.js
index f70ed3b19..704064c96 100644
--- a/ui/app/constants/error-keys.js
+++ b/ui/app/constants/error-keys.js
@@ -1,3 +1,4 @@
 export const INSUFFICIENT_FUNDS_ERROR_KEY = 'insufficientFunds'
 export const GAS_LIMIT_TOO_LOW_ERROR_KEY = 'gasLimitTooLow'
 export const TRANSACTION_ERROR_KEY = 'transactionError'
+export const TRANSACTION_NO_CONTRACT_ERROR_KEY = 'transactionErrorNoContract'
diff --git a/ui/app/helpers/transactions.util.js b/ui/app/helpers/transactions.util.js
index 64ec82225..2f4b1d095 100644
--- a/ui/app/helpers/transactions.util.js
+++ b/ui/app/helpers/transactions.util.js
@@ -125,7 +125,9 @@ export function getLatestSubmittedTxWithNonce (transactions = [], nonce = '0x0')
 
 export async function isSmartContractAddress (address) {
   const code = await global.eth.getCode(address)
-  return code && code !== '0x'
+  // Geth will return '0x', and ganache-core v2.2.1 will return '0x0'
+  const codeIsEmpty = !code || code === '0x' || code === '0x0'
+  return !codeIsEmpty
 }
 
 export function sumHexes (...args) {