Refactor checking if address is contract into a new module. (#12354)

* Refactor checking if address is contract into a new module.
Tests for new module.

Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>

* addressIsContract is an async function, use await when calling it.

Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>

* Lint fixes

Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>

* Mock ethQuery
change variable names
refactor in transaction.utils.
fix possible boolean destructiring.

Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>

* Refactor isContractAddress boolean checks.

Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>

* Lint fixes

Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>

app/scripts/controllers/transactions/index.js

@@ -39,6 +39,7 @@ import {
   CHAIN_ID_TO_GAS_LIMIT_BUFFER_MAP,
 } from '../../../../shared/constants/network';
 import { isEIP1559Transaction } from '../../../../shared/modules/transaction.utils';
+import { readAddressAsContract } from '../../../../shared/modules/contract-utils';
 import TransactionStateManager from './tx-state-manager';
 import TxGasUtil from './tx-gas-utils';
 import PendingTransactionTracker from './pending-tx-tracker';
@@ -1234,23 +1235,21 @@ export default class TransactionController extends EventEmitter {
       result = TRANSACTION_TYPES.DEPLOY_CONTRACT;
     }
 
-    let code;
+    let contractCode;
+
     if (!result) {
-      try {
+      const {
-        code = await this.query.getCode(to);
+        contractCode: resultCode,
-      } catch (e) {
+        isContractAddress,
-        code = null;
+      } = await readAddressAsContract(this.query, to);
-        log.warn(e);
-      }
 
-      const codeIsEmpty = !code || code === '0x' || code === '0x0';
+      contractCode = resultCode;
-
+      result = isContractAddress
-      result = codeIsEmpty
+        ? TRANSACTION_TYPES.CONTRACT_INTERACTION
-        ? TRANSACTION_TYPES.SIMPLE_SEND
+        : TRANSACTION_TYPES.SIMPLE_SEND;
-        : TRANSACTION_TYPES.CONTRACT_INTERACTION;
     }
 
-    return { type: result, getCodeResponse: code };
+    return { type: result, getCodeResponse: contractCode };
   }
 
   /**

shared/modules/contract-utils.js

@@ -0,0 +1,12 @@
+export const readAddressAsContract = async (ethQuery, address) => {
+  let contractCode;
+  try {
+    contractCode = await ethQuery.getCode(address);
+  } catch (e) {
+    contractCode = null;
+  }
+
+  const isContractAddress =
+    contractCode && contractCode !== '0x' && contractCode !== '0x0';
+  return { contractCode, isContractAddress };
+};

shared/modules/contract-utils.test.js

@@ -0,0 +1,30 @@
+const { readAddressAsContract } = require('./contract-utils');
+
+describe('Contract Utils', () => {
+  it('checks is an address is a contract address or not', async () => {
+    let mockEthQuery = {
+      getCode: () => {
+        return '0xa';
+      },
+    };
+    const { isContractAddress } = await readAddressAsContract(
+      mockEthQuery,
+      '0x76B4aa9Fc4d351a0062c6af8d186DF959D564A84',
+    );
+    expect(isContractAddress).toStrictEqual(true);
+
+    mockEthQuery = {
+      getCode: () => {
+        return '0x';
+      },
+    };
+
+    const {
+      isContractAddress: isNotContractAddress,
+    } = await readAddressAsContract(
+      mockEthQuery,
+      '0x76B4aa9Fc4d351a0062c6af8d186DF959D564A84',
+    );
+    expect(isNotContractAddress).toStrictEqual(false);
+  });
+});

ui/ducks/send/send.js

@@ -88,6 +88,7 @@ import {
 import { CHAIN_ID_TO_GAS_LIMIT_BUFFER_MAP } from '../../../shared/constants/network';
 import { ETH, GWEI } from '../../helpers/constants/common';
 import { TRANSACTION_ENVELOPE_TYPES } from '../../../shared/constants/transaction';
+import { readAddressAsContract } from '../../../shared/modules/contract-utils';
 // typedefs
 /**
  * @typedef {import('@reduxjs/toolkit').PayloadAction} PayloadAction
@@ -230,13 +231,12 @@ async function estimateGasLimitForSend({
       // address. If this returns 0x, 0x0 or a nullish value then the address
       // is an externally owned account (NOT a contract account). For these
       // types of transactions the gasLimit will always be 21,000 or 0x5208
-      const contractCode = Boolean(to) && (await global.eth.getCode(to));
+      const { isContractAddress } = to
-      // Geth will return '0x', and ganache-core v2.2.1 will return '0x0'
+        ? await readAddressAsContract(global.eth, to)
-      const contractCodeIsEmpty =
+        : {};
-        !contractCode || contractCode === '0x' || contractCode === '0x0';
+      if (!isContractAddress && !isNonStandardEthChain) {
-      if (contractCodeIsEmpty && !isNonStandardEthChain) {
         return GAS_LIMITS.SIMPLE;
-      } else if (contractCodeIsEmpty && isNonStandardEthChain) {
+      } else if (!isContractAddress && isNonStandardEthChain) {
         isSimpleSendOnNonStandardNetwork = true;
       }
     }

ui/helpers/utils/transactions.util.js

@@ -11,6 +11,7 @@ import {
   TRANSACTION_ENVELOPE_TYPES,
 } from '../../../shared/constants/transaction';
 import { addCurrencies } from '../../../shared/modules/conversion.utils';
+import { readAddressAsContract } from '../../../shared/modules/contract-utils';
 import fetchWithCache from './fetch-with-cache';
 
 const hstInterface = new ethers.utils.Interface(abi);
@@ -144,10 +145,8 @@ export function getLatestSubmittedTxWithNonce(
 }
 
 export async function isSmartContractAddress(address) {
-  const code = await global.eth.getCode(address);
+  const { isContractCode } = readAddressAsContract(global.eth, address);
-  // Geth will return '0x', and ganache-core v2.2.1 will return '0x0'
+  return isContractCode;
-  const codeIsEmpty = !code || code === '0x' || code === '0x0';
-  return !codeIsEmpty;
 }
 
 export function sumHexes(...args) {