Bug Fix: MetaMask#1789 and MetaMask#4525 eth.getCode() with no contract

HowardBraham · Oct 9, 2018 · 222e62d · 222e62d
1 parent db4569e
commit 222e62d
Show file tree

Hide file tree

Showing 8 changed files with 25 additions and 11 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,7 @@
 
 - Update transaction statuses when switching networks.
 - [#5470](https://github.com/MetaMask/metamask-extension/pull/5470) 100% coverage in French locale, fixed the procedure to verify proposed locale.
+- [#5283](https://github.com/MetaMask/metamask-extension/pull/5283): Fix bug when eth.getCode() called with no contract
 
 ## 4.12.0 Thursday September 27 2018
 

diff --git a/app/_locales/en/messages.json b/app/_locales/en/messages.json
@@ -1154,6 +1154,9 @@
   "transactionError": {
     "message": "Transaction Error. Exception thrown in contract code."
   },
+  "transactionErrorNoContract": {
+    "message": "Trying to call a contract function at an address that is not a 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
@@ -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,16 +59,22 @@ 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
+    if (hasRecipient) {
+      let code = await this.query.getCode(recipient)
+
+      // If there's data in the params, but there's no code, it's not a valid contract
+      // For no code, Infura will return '0x', and Ganache will return '0x0'
+      if (txParams.data && (!code || code === '0x' || code === '0x0')) {
+        throw {errorKey: TRANSACTION_NO_CONTRACT_ERROR_KEY}
+      }
+      else if (!code) {
+        txParams.gas = SIMPLE_GAS_COST // For a standard ETH send, gas is 21k max
+        txMeta.simpleSend = true // Prevents buffer addition
+        return SIMPLE_GAS_COST
+      }
     }
 
     // if not, fall back to block gasLimit

diff --git a/old-ui/app/components/pending-tx.js b/old-ui/app/components/pending-tx.js
@@ -489,7 +489,7 @@ PendingTx.prototype.verifyGasParams = function () {
 }
 
 PendingTx.prototype._notZeroOrEmptyString = function (obj) {
-  return obj !== '' && obj !== '0x0'
+  return obj !== '' && obj !== '0x0' && obj !== '0x' // The '0x' case might not ever happen, but it seems safest to protect against it
 }
 
 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
@@ -131,7 +131,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
@@ -215,7 +215,7 @@ 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') {
+    if (!code || code === '0x' || code === '0x0') { // Infura will return '0x', and Ganache will return '0x0'
       return SIMPLE_GAS_COST
     }
   } else if (selectedToken && !to) {

diff --git 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
@@ -114,7 +114,7 @@ export function getLatestSubmittedTxWithNonce (transactions = [], nonce = '0x0')
 
 export async function isSmartContractAddress (address) {
   const code = await global.eth.getCode(address)
-  return code && code !== '0x'
+  return code && code !== '0x' && code !== '0x0'; // Infura will return '0x', and Ganache will return '0x0'
 }
 
 export function sumHexes (...args) {