Stop passing a gas param to the estimateGas call initiated in the swaps controller (#9501)

* Stop passing a gas param to the estimateGas call initiated in the swaps controller timedoutGasReturn

* Stop passing gas params to timedoutGasReturn

* Lint fix

* Stop passing no longer used param to setInitialGasEstimate

app/scripts/controllers/swaps.js

@@ -154,10 +154,7 @@ export default class SwapsController {
           approvalNeeded: null,
         }))
       } else if (!isPolledRequest) {
-        const { gasLimit: approvalGas } = await this.timedoutGasReturn({
+        const { gasLimit: approvalGas } = await this.timedoutGasReturn(Object.values(newQuotes)[0].approvalNeeded)
-          ...Object.values(newQuotes)[0].approvalNeeded,
-          gas: DEFAULT_ERC20_APPROVE_GAS,
-        })
 
         newQuotes = mapValues(newQuotes, (quote) => ({
           ...quote,
@@ -255,10 +252,7 @@ export default class SwapsController {
   async getAllQuotesWithGasEstimates (quotes) {
     const quoteGasData = await Promise.all(
       Object.values(quotes).map(async (quote) => {
-        const { gasLimit, simulationFails } = await this.timedoutGasReturn({
+        const { gasLimit, simulationFails } = await this.timedoutGasReturn(quote.trade)
-          ...quote.trade,
-          gas: `0x${(quote.maxGas || MAX_GAS_LIMIT).toString(16)}`,
-        })
         return [gasLimit, simulationFails, quote.aggregator]
       }),
     )
@@ -292,7 +286,17 @@ export default class SwapsController {
         resolve({ gasLimit: null, simulationFails: true })
       }, 5000)
 
-      this.getBufferedGasLimit({ txParams: tradeTxParams }, 1)
+      // Remove gas from params that will be passed to the `estimateGas` call
+      // Including it can cause the estimate to fail if the actual gas needed
+      // exceeds the passed gas
+      const tradeTxParamsForGasEstimate = {
+        data: tradeTxParams.data,
+        from: tradeTxParams.from,
+        to: tradeTxParams.to,
+        value: tradeTxParams.value,
+      }
+
+      this.getBufferedGasLimit({ txParams: tradeTxParamsForGasEstimate }, 1)
         .then(({ gasLimit, simulationFails }) => {
           if (!gasTimedOut) {
             clearTimeout(gasTimeout)
@@ -309,7 +313,7 @@ export default class SwapsController {
     })
   }
 
-  async setInitialGasEstimate (initialAggId, baseGasEstimate) {
+  async setInitialGasEstimate (initialAggId) {
     const { swapsState } = this.store.getState()
 
     const quoteToUpdate = { ...swapsState.quotes[initialAggId] }
@@ -317,10 +321,7 @@ export default class SwapsController {
     const {
       gasLimit: newGasEstimate,
       simulationFails,
-    } = await this.timedoutGasReturn({
+    } = await this.timedoutGasReturn(quoteToUpdate.trade)
-      ...quoteToUpdate.trade,
-      gas: baseGasEstimate,
-    })
 
     if (newGasEstimate && !simulationFails) {
       const gasEstimateWithRefund = calculateGasEstimateWithRefund(quoteToUpdate.maxGas, quoteToUpdate.estimatedRefund, newGasEstimate)

test/unit/app/controllers/swaps-test.js

@@ -5,7 +5,6 @@ import { ethers } from 'ethers'
 import BigNumber from 'bignumber.js'
 import ObservableStore from 'obs-store'
 import { createTestProviderTools } from '../../../stub/provider'
-import { DEFAULT_ERC20_APPROVE_GAS } from '../../../../ui/app/helpers/constants/swaps'
 import SwapsController from '../../../../app/scripts/controllers/swaps'
 
 const MOCK_FETCH_PARAMS = {
@@ -44,6 +43,39 @@ const MOCK_QUOTES = {
     slippage: 3,
   },
 }
+const MOCK_APPROVAL_NEEDED = {
+  'data': '0x095ea7b300000000000000000000000095e6f48254609a6ee006f7d493c8e5fb97094cef0000000000000000000000000000000000000000004a817c7ffffffdabf41c00',
+  'to': '0xc02aaa39b223fe8d0a0e5c4f27ead9083c756cc2',
+  'amount': '0',
+  'from': '0x2369267687A84ac7B494daE2f1542C40E37f4455',
+  'gas': '12',
+  'gasPrice': '34',
+}
+const MOCK_QUOTES_APPROVAL_REQUIRED = {
+  [TEST_AGG_ID]: {
+    trade: {
+      data: '0x00',
+      from: '0x7F18BB4Dd92CF2404C54CBa1A9BE4A1153bdb078',
+      value: '0x17647444f166000',
+      gas: '0xe09c0',
+      gasPrice: undefined,
+      to: '0x016B4bf68d421147c06f1b8680602c5bf0Df91A8',
+    },
+    sourceAmount: '1000000000000000000000000000000000000',
+    destinationAmount: '396493201125465',
+    error: null,
+    sourceToken: '0x6b175474e89094c44da98b954eedeac495271d0f',
+    destinationToken: '0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48',
+    maxGas: 920000,
+    averageGas: 312510,
+    estimatedRefund: 343090,
+    fetchTime: 559,
+    aggregator: TEST_AGG_ID,
+    aggType: 'AGG',
+    slippage: 3,
+    approvalNeeded: MOCK_APPROVAL_NEEDED,
+  },
+}
 const MOCK_FETCH_METADATA = {
   destinationTokenInfo: {
     symbol: 'FOO',
@@ -311,7 +343,7 @@ describe('SwapsController', function () {
       })
 
       it('gets the gas limit if approval is required', async function () {
-        fetchTradesInfoStub.resolves(MOCK_QUOTES)
+        fetchTradesInfoStub.resolves(MOCK_QUOTES_APPROVAL_REQUIRED)
 
         // Ensure approval is required
         sandbox
@@ -330,9 +362,7 @@ describe('SwapsController', function () {
 
         // Mocked quotes approvalNeeded is null, so it will only be called with the gas
         assert.strictEqual(
-          timedoutGasReturnStub.calledOnceWithExactly({
+          timedoutGasReturnStub.calledOnceWithExactly(MOCK_APPROVAL_NEEDED),
-            gas: DEFAULT_ERC20_APPROVE_GAS,
-          }),
           true,
         )
       })

ui/app/ducks/swaps/swaps.js

@@ -410,7 +410,7 @@ export const fetchQuotesAndSetQuoteState = (history, inputValue, maxSlippage, me
           },
         })
 
-        dispatch(setInitialGasEstimate(selectedAggId, newSelectedQuote.maxGas))
+        dispatch(setInitialGasEstimate(selectedAggId))
       }
     } catch (e) {
       dispatch(setSwapsErrorKey(ERROR_FETCHING_QUOTES))

ui/app/store/actions.js

@@ -2265,9 +2265,9 @@ export function setSwapsErrorKey (errorKey) {
   }
 }
 
-export function setInitialGasEstimate (initialAggId, baseGasEstimate) {
+export function setInitialGasEstimate (initialAggId) {
   return async (dispatch) => {
-    await promisifiedBackground.setInitialGasEstimate(initialAggId, baseGasEstimate)
+    await promisifiedBackground.setInitialGasEstimate(initialAggId)
     await forceUpdateMetamaskState(dispatch)
   }
 }