Fix use estimate gas instead of fixed gas (#3694)

vinistevam · web-flow · commit c21235f4c73e · 2023-12-21T16:05:03.000Z
## Explanation
Fix following the full release of the core `TransactionController` in
the extension:

- Change to use estimate gas instead of fixed gas (21k) when a contract
is deployed and the `gas` is not specified.

### `@metamask/transaction-controller`

- **FIXED**: Fix use estimate gas when `to` property is not provided.
diff --git a/packages/transaction-controller/src/utils/gas.test.ts b/packages/transaction-controller/src/utils/gas.test.ts
@@ -6,7 +6,13 @@ import type EthQuery from '@metamask/eth-query';
 import { CHAIN_IDS } from '../constants';
 import type { TransactionMeta } from '../types';
 import type { UpdateGasRequest } from './gas';
-import { addGasBuffer, estimateGas, updateGas, FIXED_GAS } from './gas';
+import {
+  addGasBuffer,
+  estimateGas,
+  updateGas,
+  FIXED_GAS,
+  DEFAULT_GAS_MULTIPLIER,
+} from './gas';
 
 jest.mock('@metamask/controller-utils', () => ({
   ...jest.requireActual('@metamask/controller-utils'),
@@ -16,7 +22,6 @@ jest.mock('@metamask/controller-utils', () => ({
 const GAS_MOCK = 100;
 const BLOCK_GAS_LIMIT_MOCK = 1234567;
 const BLOCK_NUMBER_MOCK = '0x5678';
-const CODE_MOCK = '0x987';
 // TODO: Replace `any` with type
 // eslint-disable-next-line @typescript-eslint/no-explicit-any
 const ETH_QUERY_MOCK = {} as any as EthQuery;
@@ -127,11 +132,27 @@ describe('gas', () => {
         );
       });
 
+      it('to estimate if not custom network and no to parameter', async () => {
+        updateGasRequest.providerConfig.type = NetworkType.mainnet;
+        const gasEstimation = Math.ceil(GAS_MOCK * DEFAULT_GAS_MULTIPLIER);
+        delete updateGasRequest.txMeta.txParams.to;
+        mockQuery({
+          getBlockByNumberResponse: { gasLimit: toHex(BLOCK_GAS_LIMIT_MOCK) },
+          estimateGasResponse: toHex(GAS_MOCK),
+        });
+
+        await updateGas(updateGasRequest);
+
+        expect(updateGasRequest.txMeta.txParams.gas).toBe(toHex(gasEstimation));
+        expect(updateGasRequest.txMeta.originalGasEstimate).toBe(
+          updateGasRequest.txMeta.txParams.gas,
+        );
+      });
+
       it('to estimate if estimate greater than 90% of block gas limit', async () => {
         const estimatedGas = Math.ceil(BLOCK_GAS_LIMIT_MOCK * 0.9 + 10);
 
         mockQuery({
-          getCodeResponse: CODE_MOCK,
           getBlockByNumberResponse: { gasLimit: toHex(BLOCK_GAS_LIMIT_MOCK) },
           estimateGasResponse: toHex(estimatedGas),
         });
@@ -150,7 +171,6 @@ describe('gas', () => {
         const estimatedGas = Math.round(estimatedGasPadded / 1.5);
 
         mockQuery({
-          getCodeResponse: CODE_MOCK,
           getBlockByNumberResponse: { gasLimit: toHex(BLOCK_GAS_LIMIT_MOCK) },
           estimateGasResponse: toHex(estimatedGas),
         });
@@ -173,7 +193,6 @@ describe('gas', () => {
         updateGasRequest.providerConfig.chainId = CHAIN_IDS.OPTIMISM;
 
         mockQuery({
-          getCodeResponse: CODE_MOCK,
           getBlockByNumberResponse: { gasLimit: toHex(BLOCK_GAS_LIMIT_MOCK) },
           estimateGasResponse: toHex(estimatedGas),
         });
@@ -194,7 +213,6 @@ describe('gas', () => {
         const estimatedGas = Math.ceil(estimatedGasPadded / 1.5);
 
         mockQuery({
-          getCodeResponse: CODE_MOCK,
           getBlockByNumberResponse: { gasLimit: toHex(BLOCK_GAS_LIMIT_MOCK) },
           estimateGasResponse: toHex(estimatedGas),
         });
@@ -210,19 +228,6 @@ describe('gas', () => {
       });
 
       describe('to fixed value', () => {
-        it('if not custom network and no to parameter', async () => {
-          updateGasRequest.providerConfig.type = NetworkType.mainnet;
-          delete updateGasRequest.txMeta.txParams.to;
-
-          await updateGas(updateGasRequest);
-
-          expect(updateGasRequest.txMeta.txParams.gas).toBe(FIXED_GAS);
-          expect(updateGasRequest.txMeta.originalGasEstimate).toBe(
-            updateGasRequest.txMeta.txParams.gas,
-          );
-          expectEstimateGasNotCalled();
-        });
-
         it('if not custom network and to parameter and no data and no code', async () => {
           updateGasRequest.providerConfig.type = NetworkType.mainnet;
           delete updateGasRequest.txMeta.txParams.data;
@@ -264,7 +269,6 @@ describe('gas', () => {
         const fallbackGas = Math.floor(BLOCK_GAS_LIMIT_MOCK * 0.95);
 
         mockQuery({
-          getCodeResponse: CODE_MOCK,
           getBlockByNumberResponse: {
             gasLimit: toHex(BLOCK_GAS_LIMIT_MOCK),
           },
@@ -281,7 +285,6 @@ describe('gas', () => {
 
       it('sets simulationFails property', async () => {
         mockQuery({
-          getCodeResponse: CODE_MOCK,
           getBlockByNumberResponse: {
             gasLimit: toHex(BLOCK_GAS_LIMIT_MOCK),
             number: BLOCK_NUMBER_MOCK,
diff --git a/packages/transaction-controller/src/utils/gas.ts b/packages/transaction-controller/src/utils/gas.ts
@@ -167,17 +167,13 @@ async function requiresFixedGas({
     txParams: { to, data },
   } = txMeta;
 
-  if (isCustomNetwork) {
+  if (isCustomNetwork || !to || data) {
     return false;
   }
 
-  if (!to) {
-    return true;
-  }
-
   const code = await getCode(ethQuery, to);
 
-  return !data && (!code || code === '0x');
+  return !code || code === '0x';
 }
 
 async function getCode(
