diff --git a/app/scripts/controllers/swaps.js b/app/scripts/controllers/swaps.js index fd1e4824d..fc9f5823d 100644 --- a/app/scripts/controllers/swaps.js +++ b/app/scripts/controllers/swaps.js @@ -154,10 +154,7 @@ export default class SwapsController { approvalNeeded: null, })) } else if (!isPolledRequest) { - const { gasLimit: approvalGas } = await this.timedoutGasReturn({ - ...Object.values(newQuotes)[0].approvalNeeded, - gas: DEFAULT_ERC20_APPROVE_GAS, - }) + const { gasLimit: approvalGas } = await this.timedoutGasReturn(Object.values(newQuotes)[0].approvalNeeded) 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({ - ...quote.trade, - gas: `0x${(quote.maxGas || MAX_GAS_LIMIT).toString(16)}`, - }) + const { gasLimit, simulationFails } = await this.timedoutGasReturn(quote.trade) 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({ - ...quoteToUpdate.trade, - gas: baseGasEstimate, - }) + } = await this.timedoutGasReturn(quoteToUpdate.trade) if (newGasEstimate && !simulationFails) { const gasEstimateWithRefund = calculateGasEstimateWithRefund(quoteToUpdate.maxGas, quoteToUpdate.estimatedRefund, newGasEstimate) diff --git a/test/unit/app/controllers/swaps-test.js b/test/unit/app/controllers/swaps-test.js index c926182f1..171652302 100644 --- a/test/unit/app/controllers/swaps-test.js +++ b/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({ - gas: DEFAULT_ERC20_APPROVE_GAS, - }), + timedoutGasReturnStub.calledOnceWithExactly(MOCK_APPROVAL_NEEDED), true, ) }) diff --git a/ui/app/ducks/swaps/swaps.js b/ui/app/ducks/swaps/swaps.js index 3233acebc..5384e6150 100644 --- a/ui/app/ducks/swaps/swaps.js +++ b/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)) diff --git a/ui/app/store/actions.js b/ui/app/store/actions.js index 38baa67db..9fba80d02 100644 --- a/ui/app/store/actions.js +++ b/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) } }