Set a default value for code in _determineTransactionCategory (#6604)

* Set a default value for code in _determineTransactionCategory

* Adds e2e tests that fail when token txs without gas param are not properly handled.

* Adds unit tests for _determineTransactionCategory

* Base error throwing and simple gas setting in estimateTxGas on transactionCategory

2 files changed, 13 insertions, 13 deletions

diff --git a/app/scripts/controllers/transactions/index.js b/app/scripts/controllers/transactions/index.js
index dc6a043e4..79dba7833 100644
--- a/app/scripts/controllers/transactions/index.js
+++ b/app/scripts/controllers/transactions/index.js
@@ -191,7 +191,7 @@ class TransactionController extends EventEmitter {
     }
     txUtils.validateTxParams(normalizedTxParams)
     // construct txMeta
-    const { transactionCategory, code } = await this._determineTransactionCategory(txParams)
+    const { transactionCategory, getCodeResponse } = await this._determineTransactionCategory(txParams)
     let txMeta = this.txStateManager.generateTxMeta({
       txParams: normalizedTxParams,
       type: TRANSACTION_TYPE_STANDARD,
@@ -204,7 +204,7 @@ class TransactionController extends EventEmitter {
       // check whether recipient account is blacklisted
       recipientBlacklistChecker.checkAccount(txMeta.metamaskNetworkId, normalizedTxParams.to)
       // add default tx params
-      txMeta = await this.addTxGasDefaults(txMeta, code)
+      txMeta = await this.addTxGasDefaults(txMeta, getCodeResponse)
     } catch (error) {
       log.warn(error)
       txMeta.loadingDefaults = false
@@ -224,7 +224,7 @@ class TransactionController extends EventEmitter {
   @param txMeta {Object} - the txMeta object
   @returns {Promise<object>} resolves with txMeta
 */
-  async addTxGasDefaults (txMeta, code) {
+  async addTxGasDefaults (txMeta, getCodeResponse) {
     const txParams = txMeta.txParams
     // ensure value
     txParams.value = txParams.value ? ethUtil.addHexPrefix(txParams.value) : '0x0'
@@ -235,7 +235,7 @@ class TransactionController extends EventEmitter {
     }
     txParams.gasPrice = ethUtil.addHexPrefix(gasPrice.toString(16))
     // set gasLimit
-    return await this.txGasUtil.analyzeGasUsage(txMeta, code)
+    return await this.txGasUtil.analyzeGasUsage(txMeta, getCodeResponse)
   }
 
   /**
@@ -593,6 +593,7 @@ class TransactionController extends EventEmitter {
       try {
         code = await this.query.getCode(to)
       } catch (e) {
+        code = null
         log.warn(e)
       }
       // For an address with no code, geth will return '0x', and ganache-core v2.2.1 will return '0x0'
@@ -601,7 +602,7 @@ class TransactionController extends EventEmitter {
       result = codeIsEmpty ? SEND_ETHER_ACTION_KEY : CONTRACT_INTERACTION_KEY
     }
 
-    return { transactionCategory: result, code }
+    return { transactionCategory: result, getCodeResponse: code }
   }
 
   /**
diff --git a/app/scripts/controllers/transactions/tx-gas-utils.js b/app/scripts/controllers/transactions/tx-gas-utils.js
index 2dc461f48..287fb6f44 100644
--- a/app/scripts/controllers/transactions/tx-gas-utils.js
+++ b/app/scripts/controllers/transactions/tx-gas-utils.js
@@ -6,6 +6,7 @@ const {
 } = require('../../lib/util')
 const log = require('loglevel')
 const { addHexPrefix } = require('ethereumjs-util')
+const { SEND_ETHER_ACTION_KEY } = require('../../../../ui/app/helpers/constants/transactions.js')
 const SIMPLE_GAS_COST = '0x5208' // Hex for 21000, cost of a simple send.
 
 import { TRANSACTION_NO_CONTRACT_ERROR_KEY } from '../../../../ui/app/helpers/constants/error-keys'
@@ -25,14 +26,13 @@ class TxGasUtil {
 
   /**
     @param txMeta {Object} - the txMeta object
-    @param code {string} - the code at the txs address, as returned by this.query.getCode(to)
     @returns {object} the txMeta object with the gas written to the txParams
   */
-  async analyzeGasUsage (txMeta, code) {
+  async analyzeGasUsage (txMeta, getCodeResponse) {
     const block = await this.query.getBlockByNumber('latest', false)
     let estimatedGasHex
     try {
-      estimatedGasHex = await this.estimateTxGas(txMeta, block.gasLimit, code)
+      estimatedGasHex = await this.estimateTxGas(txMeta, block.gasLimit, getCodeResponse)
     } catch (err) {
       log.warn(err)
       txMeta.simulationFails = {
@@ -55,10 +55,9 @@ class TxGasUtil {
     Estimates the tx's gas usage
     @param txMeta {Object} - the txMeta object
     @param blockGasLimitHex {string} - hex string of the block's gas limit
-    @param code {string} - the code at the txs address, as returned by this.query.getCode(to)
     @returns {string} the estimated gas limit as a hex string
   */
-  async estimateTxGas (txMeta, blockGasLimitHex, code) {
+  async estimateTxGas (txMeta, blockGasLimitHex, getCodeResponse) {
     const txParams = txMeta.txParams
 
     // check if gasLimit is already specified
@@ -75,9 +74,9 @@ class TxGasUtil {
     // see if we can set the gas based on the recipient
     if (hasRecipient) {
       // 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'
+      const categorizedAsSimple = txMeta.transactionCategory === SEND_ETHER_ACTION_KEY
 
-      if (codeIsEmpty) {
+      if (categorizedAsSimple) {
         // 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')
@@ -85,7 +84,7 @@ class TxGasUtil {
           err.errorKey = TRANSACTION_NO_CONTRACT_ERROR_KEY
 
           // set the response on the error so that we can see in logs what the actual response was
-          err.getCodeResponse = code
+          err.getCodeResponse = getCodeResponse
           throw err
         }