From f4a2ed1a14e1d958bd91fd3ae626b98e6a2e08f3 Mon Sep 17 00:00:00 2001 From: David Walsh Date: Tue, 8 Dec 2020 12:49:19 -0600 Subject: [PATCH 01/10] Fix 10010 - Fetch swap quote refresh time from API --- ui/app/ducks/swaps/swaps.js | 21 ++++++++++++++- .../swaps/countdown-timer/countdown-timer.js | 15 ++++++----- ui/app/pages/swaps/swaps.util.js | 27 ++++++++++++++----- ui/app/pages/swaps/view-quote/view-quote.js | 17 +++++++++--- 4 files changed, 64 insertions(+), 16 deletions(-) diff --git a/ui/app/ducks/swaps/swaps.js b/ui/app/ducks/swaps/swaps.js index dd7fab0a1baf..6c89cc437062 100644 --- a/ui/app/ducks/swaps/swaps.js +++ b/ui/app/ducks/swaps/swaps.js @@ -34,6 +34,7 @@ import { import { fetchSwapsFeatureLiveness, fetchSwapsGasPrices, + fetchSwapsQuoteRefreshTime, } from '../../pages/swaps/swaps.util' import { calcGasTotal } from '../../pages/send/send.utils' import { @@ -85,6 +86,8 @@ const initialState = { priceEstimatesLastRetrieved: 0, fallBackPrice: null, }, + // This will eventually be overriden by a property provided by the MetaSwap API + swapsQuoteRefreshTime: 0, } const slice = createSlice({ @@ -150,6 +153,9 @@ const slice = createSlice({ retrievedFallbackSwapsGasPrice: (state, action) => { state.customGas.fallBackPrice = action.payload }, + setSwapsQuoteRefreshTime: (state, action) => { + state.swapsQuoteRefreshTime = action.payload + }, }, }) @@ -264,6 +270,9 @@ export const getApproveTxId = (state) => state.metamask.swapsState.approveTxId export const getTradeTxId = (state) => state.metamask.swapsState.tradeTxId +export const getSwapsQuoteRefreshTime = (state) => + state.swaps.swapsQuoteRefreshTime + export const getUsedQuote = (state) => getSelectedQuote(state) || getTopQuote(state) @@ -307,6 +316,7 @@ const { swapCustomGasModalLimitEdited, retrievedFallbackSwapsGasPrice, swapCustomGasModalClosed, + setSwapsQuoteRefreshTime, } = actions export { @@ -323,11 +333,17 @@ export { swapCustomGasModalClosed, } +export function fetchMetaSwapsQuoteRefreshTime() { + return async (dispatch) => { + const quoteRefreshTime = await fetchSwapsQuoteRefreshTime() + dispatch(setSwapsQuoteRefreshTime(quoteRefreshTime)) + } +} + export const navigateBackToBuildQuote = (history) => { return async (dispatch) => { // TODO: Ensure any fetch in progress is cancelled dispatch(navigatedBackToBuildQuote()) - history.push(BUILD_QUOTE_ROUTE) } } @@ -504,9 +520,12 @@ export const fetchQuotesAndSetQuoteState = ( const gasPriceFetchPromise = dispatch(fetchAndSetSwapsGasPriceInfo()) + const quoteRefreshTimePromise = dispatch(fetchMetaSwapsQuoteRefreshTime()) + const [[fetchedQuotes, selectedAggId]] = await Promise.all([ fetchAndSetQuotesPromise, gasPriceFetchPromise, + quoteRefreshTimePromise, ]) if (Object.values(fetchedQuotes)?.length === 0) { diff --git a/ui/app/pages/swaps/countdown-timer/countdown-timer.js b/ui/app/pages/swaps/countdown-timer/countdown-timer.js index 9a7362536fe1..393a51849571 100644 --- a/ui/app/pages/swaps/countdown-timer/countdown-timer.js +++ b/ui/app/pages/swaps/countdown-timer/countdown-timer.js @@ -1,11 +1,11 @@ import React, { useState, useEffect, useContext, useRef } from 'react' +import { useSelector } from 'react-redux' import PropTypes from 'prop-types' import classnames from 'classnames' import { Duration } from 'luxon' import { I18nContext } from '../../../contexts/i18n' import InfoTooltip from '../../../components/ui/info-tooltip' - -const TIMER_BASE = 60000 +import { getSwapsQuoteRefreshTime } from '../../../ducks/swaps/swaps' // Return the mm:ss start time of the countdown timer. // If time has elapsed between `timeStarted` the time current time, @@ -31,7 +31,7 @@ function timeBelowWarningTime(timer, warningTime) { export default function CountdownTimer({ timeStarted, timeOnly, - timerBase = TIMER_BASE, + timerBase, warningTime, labelKey, infoTooltipLabelKey, @@ -40,9 +40,12 @@ export default function CountdownTimer({ const intervalRef = useRef() const initialTimeStartedRef = useRef() + const swapsQuoteRefreshTime = useSelector(getSwapsQuoteRefreshTime) + const timerStart = Number(timerBase) || swapsQuoteRefreshTime + const [currentTime, setCurrentTime] = useState(() => Date.now()) const [timer, setTimer] = useState(() => - getNewTimer(currentTime, timeStarted, timerBase), + getNewTimer(currentTime, timeStarted, timerStart), ) useEffect(() => { @@ -67,14 +70,14 @@ export default function CountdownTimer({ initialTimeStartedRef.current = timeStarted const newCurrentTime = Date.now() setCurrentTime(newCurrentTime) - setTimer(getNewTimer(newCurrentTime, timeStarted, timerBase)) + setTimer(getNewTimer(newCurrentTime, timeStarted, timerStart)) clearInterval(intervalRef.current) intervalRef.current = setInterval(() => { setTimer(decreaseTimerByOne) }, 1000) } - }, [timeStarted, timer, timerBase]) + }, [timeStarted, timer, timerStart]) const formattedTimer = Duration.fromMillis(timer).toFormat('m:ss') let time diff --git a/ui/app/pages/swaps/swaps.util.js b/ui/app/pages/swaps/swaps.util.js index e5248a76e11d..d0b080348f70 100644 --- a/ui/app/pages/swaps/swaps.util.js +++ b/ui/app/pages/swaps/swaps.util.js @@ -24,20 +24,24 @@ const TOKEN_TRANSFER_LOG_TOPIC_HASH = const CACHE_REFRESH_ONE_HOUR = 3600000 +const METASWAP_API_HOST = 'https://api.metaswap.codefi.network' + const getBaseApi = function (type) { switch (type) { case 'trade': - return `https://api.metaswap.codefi.network/trades?` + return `${METASWAP_API_HOST}/trades?` case 'tokens': - return `https://api.metaswap.codefi.network/tokens` + return `${METASWAP_API_HOST}/tokens` case 'topAssets': - return `https://api.metaswap.codefi.network/topAssets` + return `${METASWAP_API_HOST}/topAssets` case 'featureFlag': - return `https://api.metaswap.codefi.network/featureFlag` + return `${METASWAP_API_HOST}/featureFlag` case 'aggregatorMetadata': - return `https://api.metaswap.codefi.network/aggregatorMetadata` + return `${METASWAP_API_HOST}/aggregatorMetadata` case 'gasPrices': - return `https://api.metaswap.codefi.network/gasPrices` + return `${METASWAP_API_HOST}/gasPrices` + case 'refreshTime': + return `${METASWAP_API_HOST}/quoteRefreshRate` default: throw new Error('getBaseApi requires an api call type') } @@ -328,6 +332,17 @@ export async function fetchSwapsFeatureLiveness() { return status?.active } +export async function fetchSwapsQuoteRefreshTime() { + const response = await fetchWithCache( + getBaseApi('refreshTime'), + { method: 'GET' }, + { cacheRefreshTime: 600000 }, + ) + + // We presently use milliseconds in the UI + return response?.seconds * 1000 +} + export async function fetchTokenPrice(address) { const query = `contract_addresses=${address}&vs_currencies=eth` diff --git a/ui/app/pages/swaps/view-quote/view-quote.js b/ui/app/pages/swaps/view-quote/view-quote.js index 3c5f7b96ac53..bdceee91fd40 100644 --- a/ui/app/pages/swaps/view-quote/view-quote.js +++ b/ui/app/pages/swaps/view-quote/view-quote.js @@ -28,6 +28,7 @@ import { signAndSendTransactions, getBackgroundSwapRouteState, swapsQuoteSelected, + getSwapsQuoteRefreshTime, } from '../../../ducks/swaps/swaps' import { conversionRateSelector, @@ -114,6 +115,7 @@ export default function ViewQuote() { const topQuote = useSelector(getTopQuote) const usedQuote = selectedQuote || topQuote const tradeValue = usedQuote?.trade?.value ?? '0x0' + const swapsQuoteRefreshTime = useSelector(getSwapsQuoteRefreshTime) const { isBestQuote } = usedQuote @@ -265,14 +267,23 @@ export default function ViewQuote() { useEffect(() => { const currentTime = Date.now() const timeSinceLastFetched = currentTime - quotesLastFetched - if (timeSinceLastFetched > 60000 && !dispatchedSafeRefetch) { + if ( + timeSinceLastFetched > swapsQuoteRefreshTime && + !dispatchedSafeRefetch + ) { setDispatchedSafeRefetch(true) dispatch(safeRefetchQuotes()) - } else if (timeSinceLastFetched > 60000) { + } else if (timeSinceLastFetched > swapsQuoteRefreshTime) { dispatch(setSwapsErrorKey(QUOTES_EXPIRED_ERROR)) history.push(SWAPS_ERROR_ROUTE) } - }, [quotesLastFetched, dispatchedSafeRefetch, dispatch, history]) + }, [ + quotesLastFetched, + dispatchedSafeRefetch, + dispatch, + history, + swapsQuoteRefreshTime, + ]) useEffect(() => { if (!originalApproveAmount && approveAmount) { From afdb8dd59717779e1093592452446cf5a9f3f6bf Mon Sep 17 00:00:00 2001 From: David Walsh Date: Fri, 11 Dec 2020 11:57:09 -0600 Subject: [PATCH 02/10] Move refresh time to the controller --- app/scripts/controllers/swaps.js | 38 +++++++++++++++++++++++-- test/unit/app/controllers/swaps-test.js | 8 +++++- ui/app/ducks/swaps/swaps.js | 16 +++-------- ui/app/store/actions.js | 7 +++++ 4 files changed, 53 insertions(+), 16 deletions(-) diff --git a/app/scripts/controllers/swaps.js b/app/scripts/controllers/swaps.js index c7691dffe5d4..2abf2d1d840a 100644 --- a/app/scripts/controllers/swaps.js +++ b/app/scripts/controllers/swaps.js @@ -17,6 +17,7 @@ import { import { fetchTradesInfo as defaultFetchTradesInfo, fetchSwapsFeatureLiveness as defaultFetchSwapsFeatureLiveness, + fetchSwapsQuoteRefreshTime as defaultFetchSwapsQuoteRefreshTime, } from '../../../ui/app/pages/swaps/swaps.util' const METASWAP_ADDRESS = '0x881d40237659c251811cec9c364ef91dc08d300c' @@ -42,8 +43,13 @@ function calculateGasEstimateWithRefund( return gasEstimateWithRefund } -// This is the amount of time to wait, after successfully fetching quotes and their gas estimates, before fetching for new quotes -const QUOTE_POLLING_INTERVAL = 50 * 1000 +// If for any reason the MetaSwap API fails to provide a refresh time, +// provide a reasonable fallback to avoid further errors +const FALLBACK_QUOTE_REFRESH_TIME = 60000 + +// This is the amount of time to wait, after successfully fetching quotes +// and their gas estimates, before fetching for new quotes +const QUOTE_POLLING_DIFFERENCE_INTERVAL = 10 * 1000 const initialState = { swapsState: { @@ -61,6 +67,7 @@ const initialState = { topAggId: null, routeState: '', swapsFeatureIsLive: false, + swapsQuoteRefreshTime: 0, }, } @@ -73,6 +80,7 @@ export default class SwapsController { tokenRatesStore, fetchTradesInfo = defaultFetchTradesInfo, fetchSwapsFeatureLiveness = defaultFetchSwapsFeatureLiveness, + fetchSwapsQuoteRefreshTime = defaultFetchSwapsQuoteRefreshTime, }) { this.store = new ObservableStore({ swapsState: { ...initialState.swapsState }, @@ -80,6 +88,7 @@ export default class SwapsController { this._fetchTradesInfo = fetchTradesInfo this._fetchSwapsFeatureLiveness = fetchSwapsFeatureLiveness + this._fetchSwapsQuoteRefreshTime = fetchSwapsQuoteRefreshTime this.getBufferedGasLimit = getBufferedGasLimit this.tokenRatesStore = tokenRatesStore @@ -99,6 +108,18 @@ export default class SwapsController { }) this._setupSwapsLivenessFetching() + this._setSwapsQuoteRefreshTime() + } + + // Sets the refresh rate for quote updates from the MetaSwap API + async _setSwapsQuoteRefreshTime() { + let refreshTime = FALLBACK_QUOTE_REFRESH_TIME + try { + refreshTime = await this._fetchSwapsQuoteRefreshTime() + } catch (e) { + console.error('Request for swaps quote refresh time failed: ', e) + } + this.setSwapsQuoteRefreshTime(refreshTime) } // Once quotes are fetched, we poll for new ones to keep the quotes up to date. Market and aggregator contract conditions can change fast enough @@ -106,6 +127,8 @@ export default class SwapsController { // state. These stored parameters are used on subsequent calls made during polling. // Note: we stop polling after 3 requests, until new quotes are explicitly asked for. The logic that enforces that maximum is in the body of fetchAndSetQuotes pollForNewQuotes() { + const { swapsState: { swapsQuoteRefreshTime } } = this.store.getState() + this.pollingTimeout = setTimeout(() => { const { swapsState } = this.store.getState() this.fetchAndSetQuotes( @@ -113,7 +136,7 @@ export default class SwapsController { swapsState.fetchParams?.metaData, true, ) - }, QUOTE_POLLING_INTERVAL) + }, swapsQuoteRefreshTime - QUOTE_POLLING_DIFFERENCE_INTERVAL) } stopPollingForQuotes() { @@ -413,6 +436,13 @@ export default class SwapsController { }) } + setSwapsQuoteRefreshTime(swapsQuoteRefreshTime) { + const { swapsState } = this.store.getState() + this.store.updateState({ + swapsState: { ...swapsState, swapsQuoteRefreshTime }, + }) + } + resetPostFetchState() { const { swapsState } = this.store.getState() @@ -422,6 +452,7 @@ export default class SwapsController { tokens: swapsState.tokens, fetchParams: swapsState.fetchParams, swapsFeatureIsLive: swapsState.swapsFeatureIsLive, + swapsQuoteRefreshTime: swapsState.swapsQuoteRefreshTime, }, }) clearTimeout(this.pollingTimeout) @@ -435,6 +466,7 @@ export default class SwapsController { ...initialState.swapsState, tokens: swapsState.tokens, swapsFeatureIsLive: swapsState.swapsFeatureIsLive, + swapsQuoteRefreshTime: swapsState.swapsQuoteRefreshTime, }, }) clearTimeout(this.pollingTimeout) diff --git a/test/unit/app/controllers/swaps-test.js b/test/unit/app/controllers/swaps-test.js index 775c53d9f884..fb6399474913 100644 --- a/test/unit/app/controllers/swaps-test.js +++ b/test/unit/app/controllers/swaps-test.js @@ -121,12 +121,14 @@ const EMPTY_INIT_STATE = { topAggId: null, routeState: '', swapsFeatureIsLive: false, + swapsQuoteRefreshTime: 0, }, } const sandbox = sinon.createSandbox() const fetchTradesInfoStub = sandbox.stub() const fetchSwapsFeatureLivenessStub = sandbox.stub() +const fetchSwapsQuoteRefreshTimeStub = sandbox.stub() describe('SwapsController', function () { let provider @@ -140,6 +142,7 @@ describe('SwapsController', function () { tokenRatesStore: MOCK_TOKEN_RATES_STORE, fetchTradesInfo: fetchTradesInfoStub, fetchSwapsFeatureLiveness: fetchSwapsFeatureLivenessStub, + fetchSwapsQuoteRefreshTime: fetchSwapsQuoteRefreshTimeStub, }) } @@ -805,6 +808,7 @@ describe('SwapsController', function () { assert.deepStrictEqual(swapsState, { ...EMPTY_INIT_STATE.swapsState, tokens: old.tokens, + swapsQuoteRefreshTime: old.swapsQuoteRefreshTime, }) }) @@ -850,8 +854,9 @@ describe('SwapsController', function () { const tokens = 'test' const fetchParams = 'test' const swapsFeatureIsLive = false + const swapsQuoteRefreshTime = 0 swapsController.store.updateState({ - swapsState: { tokens, fetchParams, swapsFeatureIsLive }, + swapsState: { tokens, fetchParams, swapsFeatureIsLive, swapsQuoteRefreshTime, }, }) swapsController.resetPostFetchState() @@ -862,6 +867,7 @@ describe('SwapsController', function () { tokens, fetchParams, swapsFeatureIsLive, + swapsQuoteRefreshTime, }) }) }) diff --git a/ui/app/ducks/swaps/swaps.js b/ui/app/ducks/swaps/swaps.js index 6c89cc437062..67b93dd8db01 100644 --- a/ui/app/ducks/swaps/swaps.js +++ b/ui/app/ducks/swaps/swaps.js @@ -21,6 +21,7 @@ import { updateTransaction, resetBackgroundSwapsState, setSwapsLiveness, + setSwapsQuoteRefreshTime, setSelectedQuoteAggId, setSwapsTxGasLimit, } from '../../store/actions' @@ -86,8 +87,6 @@ const initialState = { priceEstimatesLastRetrieved: 0, fallBackPrice: null, }, - // This will eventually be overriden by a property provided by the MetaSwap API - swapsQuoteRefreshTime: 0, } const slice = createSlice({ @@ -153,9 +152,6 @@ const slice = createSlice({ retrievedFallbackSwapsGasPrice: (state, action) => { state.customGas.fallBackPrice = action.payload }, - setSwapsQuoteRefreshTime: (state, action) => { - state.swapsQuoteRefreshTime = action.payload - }, }, }) @@ -230,6 +226,9 @@ const getSwapsState = (state) => state.metamask.swapsState export const getSwapsFeatureLiveness = (state) => state.metamask.swapsState.swapsFeatureIsLive +export const getSwapsQuoteRefreshTime = (state) => + state.metamask.swapsState.swapsQuoteRefreshTime + export const getBackgroundSwapRouteState = (state) => state.metamask.swapsState.routeState @@ -270,9 +269,6 @@ export const getApproveTxId = (state) => state.metamask.swapsState.approveTxId export const getTradeTxId = (state) => state.metamask.swapsState.tradeTxId -export const getSwapsQuoteRefreshTime = (state) => - state.swaps.swapsQuoteRefreshTime - export const getUsedQuote = (state) => getSelectedQuote(state) || getTopQuote(state) @@ -316,7 +312,6 @@ const { swapCustomGasModalLimitEdited, retrievedFallbackSwapsGasPrice, swapCustomGasModalClosed, - setSwapsQuoteRefreshTime, } = actions export { @@ -520,12 +515,9 @@ export const fetchQuotesAndSetQuoteState = ( const gasPriceFetchPromise = dispatch(fetchAndSetSwapsGasPriceInfo()) - const quoteRefreshTimePromise = dispatch(fetchMetaSwapsQuoteRefreshTime()) - const [[fetchedQuotes, selectedAggId]] = await Promise.all([ fetchAndSetQuotesPromise, gasPriceFetchPromise, - quoteRefreshTimePromise, ]) if (Object.values(fetchedQuotes)?.length === 0) { diff --git a/ui/app/store/actions.js b/ui/app/store/actions.js index 57accb9938d7..2243804a3024 100644 --- a/ui/app/store/actions.js +++ b/ui/app/store/actions.js @@ -2241,6 +2241,13 @@ export function setSwapsLiveness(swapsFeatureIsLive) { } } +export function setSwapsQuoteRefreshTime(refreshTime) { + return async (dispatch) => { + await promisifiedBackground.setSwapsQuoteRefreshTime(refreshTime) + await forceUpdateMetamaskState(dispatch) + } +} + export function fetchAndSetQuotes(fetchParams, fetchParamsMetaData) { return async (dispatch) => { const [ From ff19c561e6a840cef03ec7cd7358363a1dffb41a Mon Sep 17 00:00:00 2001 From: David Walsh Date: Mon, 14 Dec 2020 13:56:42 -0600 Subject: [PATCH 03/10] Lazy-fetch the refresh time --- app/scripts/controllers/swaps.js | 12 ++++++++---- test/unit/app/controllers/swaps-test.js | 7 ++++++- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/app/scripts/controllers/swaps.js b/app/scripts/controllers/swaps.js index 2abf2d1d840a..85c032dd0624 100644 --- a/app/scripts/controllers/swaps.js +++ b/app/scripts/controllers/swaps.js @@ -108,15 +108,15 @@ export default class SwapsController { }) this._setupSwapsLivenessFetching() - this._setSwapsQuoteRefreshTime() } // Sets the refresh rate for quote updates from the MetaSwap API async _setSwapsQuoteRefreshTime() { - let refreshTime = FALLBACK_QUOTE_REFRESH_TIME + let refreshTime try { refreshTime = await this._fetchSwapsQuoteRefreshTime() } catch (e) { + refreshTime = FALLBACK_QUOTE_REFRESH_TIME console.error('Request for swaps quote refresh time failed: ', e) } this.setSwapsQuoteRefreshTime(refreshTime) @@ -127,7 +127,9 @@ export default class SwapsController { // state. These stored parameters are used on subsequent calls made during polling. // Note: we stop polling after 3 requests, until new quotes are explicitly asked for. The logic that enforces that maximum is in the body of fetchAndSetQuotes pollForNewQuotes() { - const { swapsState: { swapsQuoteRefreshTime } } = this.store.getState() + const { + swapsState: { swapsQuoteRefreshTime }, + } = this.store.getState() this.pollingTimeout = setTimeout(() => { const { swapsState } = this.store.getState() @@ -151,7 +153,6 @@ export default class SwapsController { if (!fetchParams) { return null } - // Every time we get a new request that is not from the polling, we reset the poll count so we can poll for up to three more sets of quotes with these new params. if (!isPolledRequest) { this.pollCount = 0 @@ -164,6 +165,9 @@ export default class SwapsController { this.setSwapsErrorKey('') } + // Ensure we have a quotes refresh rate + await this._setSwapsQuoteRefreshTime() + const indexOfCurrentCall = this.indexOfNewestCallInFlight + 1 this.indexOfNewestCallInFlight = indexOfCurrentCall diff --git a/test/unit/app/controllers/swaps-test.js b/test/unit/app/controllers/swaps-test.js index fb6399474913..48e8fdcb4f8b 100644 --- a/test/unit/app/controllers/swaps-test.js +++ b/test/unit/app/controllers/swaps-test.js @@ -856,7 +856,12 @@ describe('SwapsController', function () { const swapsFeatureIsLive = false const swapsQuoteRefreshTime = 0 swapsController.store.updateState({ - swapsState: { tokens, fetchParams, swapsFeatureIsLive, swapsQuoteRefreshTime, }, + swapsState: { + tokens, + fetchParams, + swapsFeatureIsLive, + swapsQuoteRefreshTime, + }, }) swapsController.resetPostFetchState() From d3ec4ec460a101e65e6ef4e5c068a18096034b8f Mon Sep 17 00:00:00 2001 From: David Walsh Date: Mon, 14 Dec 2020 16:39:26 -0600 Subject: [PATCH 04/10] Cleanup --- app/scripts/controllers/swaps.js | 30 ++++++++++++------------ ui/app/ducks/swaps/swaps.js | 9 ------- ui/app/helpers/utils/fetch-with-cache.js | 1 + ui/app/pages/swaps/swaps.util.js | 8 +++++-- 4 files changed, 22 insertions(+), 26 deletions(-) diff --git a/app/scripts/controllers/swaps.js b/app/scripts/controllers/swaps.js index 85c032dd0624..fc625d43e9ef 100644 --- a/app/scripts/controllers/swaps.js +++ b/app/scripts/controllers/swaps.js @@ -29,6 +29,14 @@ const MAX_GAS_LIMIT = 2500000 // 3 seems to be an appropriate balance of giving users the time they need when MetaMask is not left idle, and turning polling off when it is. const POLL_COUNT_LIMIT = 3 +// If for any reason the MetaSwap API fails to provide a refresh time, +// provide a reasonable fallback to avoid further errors +const FALLBACK_QUOTE_REFRESH_TIME = 60000 + +// This is the amount of time to wait, after successfully fetching quotes +// and their gas estimates, before fetching for new quotes +const QUOTE_POLLING_DIFFERENCE_INTERVAL = 10 * 1000 + function calculateGasEstimateWithRefund( maxGas = MAX_GAS_LIMIT, estimatedRefund = 0, @@ -43,14 +51,6 @@ function calculateGasEstimateWithRefund( return gasEstimateWithRefund } -// If for any reason the MetaSwap API fails to provide a refresh time, -// provide a reasonable fallback to avoid further errors -const FALLBACK_QUOTE_REFRESH_TIME = 60000 - -// This is the amount of time to wait, after successfully fetching quotes -// and their gas estimates, before fetching for new quotes -const QUOTE_POLLING_DIFFERENCE_INTERVAL = 10 * 1000 - const initialState = { swapsState: { quotes: {}, @@ -112,11 +112,11 @@ export default class SwapsController { // Sets the refresh rate for quote updates from the MetaSwap API async _setSwapsQuoteRefreshTime() { - let refreshTime + // Default to fallback time unless API returns valid response + let refreshTime = FALLBACK_QUOTE_REFRESH_TIME try { - refreshTime = await this._fetchSwapsQuoteRefreshTime() + refreshTime = await this._fetchSwapsQuoteRefreshTime(FALLBACK_QUOTE_REFRESH_TIME) } catch (e) { - refreshTime = FALLBACK_QUOTE_REFRESH_TIME console.error('Request for swaps quote refresh time failed: ', e) } this.setSwapsQuoteRefreshTime(refreshTime) @@ -165,13 +165,13 @@ export default class SwapsController { this.setSwapsErrorKey('') } - // Ensure we have a quotes refresh rate - await this._setSwapsQuoteRefreshTime() - const indexOfCurrentCall = this.indexOfNewestCallInFlight + 1 this.indexOfNewestCallInFlight = indexOfCurrentCall - let newQuotes = await this._fetchTradesInfo(fetchParams) + let [newQuotes] = await Promise.all([ + this._fetchTradesInfo(fetchParams), + this._setSwapsQuoteRefreshTime(), + ]) newQuotes = mapValues(newQuotes, (quote) => ({ ...quote, diff --git a/ui/app/ducks/swaps/swaps.js b/ui/app/ducks/swaps/swaps.js index 67b93dd8db01..48ee7fe5ef70 100644 --- a/ui/app/ducks/swaps/swaps.js +++ b/ui/app/ducks/swaps/swaps.js @@ -21,7 +21,6 @@ import { updateTransaction, resetBackgroundSwapsState, setSwapsLiveness, - setSwapsQuoteRefreshTime, setSelectedQuoteAggId, setSwapsTxGasLimit, } from '../../store/actions' @@ -35,7 +34,6 @@ import { import { fetchSwapsFeatureLiveness, fetchSwapsGasPrices, - fetchSwapsQuoteRefreshTime, } from '../../pages/swaps/swaps.util' import { calcGasTotal } from '../../pages/send/send.utils' import { @@ -328,13 +326,6 @@ export { swapCustomGasModalClosed, } -export function fetchMetaSwapsQuoteRefreshTime() { - return async (dispatch) => { - const quoteRefreshTime = await fetchSwapsQuoteRefreshTime() - dispatch(setSwapsQuoteRefreshTime(quoteRefreshTime)) - } -} - export const navigateBackToBuildQuote = (history) => { return async (dispatch) => { // TODO: Ensure any fetch in progress is cancelled diff --git a/ui/app/helpers/utils/fetch-with-cache.js b/ui/app/helpers/utils/fetch-with-cache.js index 351ee4ab9497..faabc2cbe9a2 100644 --- a/ui/app/helpers/utils/fetch-with-cache.js +++ b/ui/app/helpers/utils/fetch-with-cache.js @@ -48,6 +48,7 @@ const fetchWithCache = async ( cachedResponse: responseJson, cachedTime: currentTime, } + cachedFetch[url] = cacheEntry await setStorageItem('cachedFetch', cachedFetch) return responseJson diff --git a/ui/app/pages/swaps/swaps.util.js b/ui/app/pages/swaps/swaps.util.js index d0b080348f70..8fdf45b2d941 100644 --- a/ui/app/pages/swaps/swaps.util.js +++ b/ui/app/pages/swaps/swaps.util.js @@ -332,7 +332,7 @@ export async function fetchSwapsFeatureLiveness() { return status?.active } -export async function fetchSwapsQuoteRefreshTime() { +export async function fetchSwapsQuoteRefreshTime(defaultValue = 0) { const response = await fetchWithCache( getBaseApi('refreshTime'), { method: 'GET' }, @@ -340,7 +340,11 @@ export async function fetchSwapsQuoteRefreshTime() { ) // We presently use milliseconds in the UI - return response?.seconds * 1000 + if (response?.seconds) { + return response.seconds * 1000 + } + + return defaultValue } export async function fetchTokenPrice(address) { From 0a15365c4dd59337126845867fb201d15cc3af10 Mon Sep 17 00:00:00 2001 From: David Walsh Date: Mon, 14 Dec 2020 16:57:54 -0600 Subject: [PATCH 05/10] Add metric error tracking --- app/scripts/controllers/swaps.js | 28 +++++++++++++++++++++------- app/scripts/metamask-controller.js | 3 +++ ui/app/store/actions.js | 7 ------- 3 files changed, 24 insertions(+), 14 deletions(-) diff --git a/app/scripts/controllers/swaps.js b/app/scripts/controllers/swaps.js index fc625d43e9ef..e16957207a89 100644 --- a/app/scripts/controllers/swaps.js +++ b/app/scripts/controllers/swaps.js @@ -78,6 +78,7 @@ export default class SwapsController { provider, getProviderConfig, tokenRatesStore, + trackMetaMetricsEvent, fetchTradesInfo = defaultFetchTradesInfo, fetchSwapsFeatureLiveness = defaultFetchSwapsFeatureLiveness, fetchSwapsQuoteRefreshTime = defaultFetchSwapsQuoteRefreshTime, @@ -89,6 +90,7 @@ export default class SwapsController { this._fetchTradesInfo = fetchTradesInfo this._fetchSwapsFeatureLiveness = fetchSwapsFeatureLiveness this._fetchSwapsQuoteRefreshTime = fetchSwapsQuoteRefreshTime + this._trackEvent = trackMetaMetricsEvent this.getBufferedGasLimit = getBufferedGasLimit this.tokenRatesStore = tokenRatesStore @@ -113,13 +115,28 @@ export default class SwapsController { // Sets the refresh rate for quote updates from the MetaSwap API async _setSwapsQuoteRefreshTime() { // Default to fallback time unless API returns valid response - let refreshTime = FALLBACK_QUOTE_REFRESH_TIME + let swapsQuoteRefreshTime = FALLBACK_QUOTE_REFRESH_TIME try { - refreshTime = await this._fetchSwapsQuoteRefreshTime(FALLBACK_QUOTE_REFRESH_TIME) + swapsQuoteRefreshTime = await this._fetchSwapsQuoteRefreshTime( + FALLBACK_QUOTE_REFRESH_TIME, + ) } catch (e) { + this._trackEvent({ + category: 'Swaps', + event: 'quote_refresh_api_error', + properties: { + error_message: e.message, + }, + }) console.error('Request for swaps quote refresh time failed: ', e) } - this.setSwapsQuoteRefreshTime(refreshTime) + + const { + swapsState + } = this.store.getState() + this.store.updateState({ + swapsState: { ...swapsState, swapsQuoteRefreshTime }, + }) } // Once quotes are fetched, we poll for new ones to keep the quotes up to date. Market and aggregator contract conditions can change fast enough @@ -441,10 +458,7 @@ export default class SwapsController { } setSwapsQuoteRefreshTime(swapsQuoteRefreshTime) { - const { swapsState } = this.store.getState() - this.store.updateState({ - swapsState: { ...swapsState, swapsQuoteRefreshTime }, - }) + } resetPostFetchState() { diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index f86bb14681ec..92d01caf820a 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -352,6 +352,9 @@ export default class MetamaskController extends EventEmitter { this.networkController, ), tokenRatesStore: this.tokenRatesController.store, + trackMetaMetricsEvent: this.metaMetricsController.trackEvent.bind( + this.metaMetricsController, + ), }) // ensure isClientOpenAndUnlocked is updated when memState updates diff --git a/ui/app/store/actions.js b/ui/app/store/actions.js index 2243804a3024..57accb9938d7 100644 --- a/ui/app/store/actions.js +++ b/ui/app/store/actions.js @@ -2241,13 +2241,6 @@ export function setSwapsLiveness(swapsFeatureIsLive) { } } -export function setSwapsQuoteRefreshTime(refreshTime) { - return async (dispatch) => { - await promisifiedBackground.setSwapsQuoteRefreshTime(refreshTime) - await forceUpdateMetamaskState(dispatch) - } -} - export function fetchAndSetQuotes(fetchParams, fetchParamsMetaData) { return async (dispatch) => { const [ From 7aceaac14db9f58974f552a56d883e9cfe78ecbd Mon Sep 17 00:00:00 2001 From: David Walsh Date: Tue, 15 Dec 2020 09:39:57 -0600 Subject: [PATCH 06/10] Add mock return for refresh time --- test/unit/app/controllers/swaps-test.js | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/test/unit/app/controllers/swaps-test.js b/test/unit/app/controllers/swaps-test.js index 48e8fdcb4f8b..3830be6daec9 100644 --- a/test/unit/app/controllers/swaps-test.js +++ b/test/unit/app/controllers/swaps-test.js @@ -642,15 +642,15 @@ describe('SwapsController', function () { const quotes = await swapsController.fetchAndSetQuotes(undefined) assert.strictEqual(quotes, null) }) - it('calls fetchTradesInfo with the given fetchParams and returns the correct quotes', async function () { fetchTradesInfoStub.resolves(getMockQuotes()) + fetchSwapsQuoteRefreshTimeStub.resolves(getMockQuoteRefreshTime()) // Make it so approval is not required sandbox .stub(swapsController, '_getERC20Allowance') .resolves(ethers.BigNumber.from(1)) - + const [newQuotes] = await swapsController.fetchAndSetQuotes( MOCK_FETCH_PARAMS, MOCK_FETCH_METADATA, @@ -685,9 +685,9 @@ describe('SwapsController', function () { true, ) }) - it('performs the allowance check', async function () { fetchTradesInfoStub.resolves(getMockQuotes()) + fetchSwapsQuoteRefreshTimeStub.resolves(getMockQuoteRefreshTime()) // Make it so approval is not required const allowanceStub = sandbox @@ -710,6 +710,7 @@ describe('SwapsController', function () { it('gets the gas limit if approval is required', async function () { fetchTradesInfoStub.resolves(MOCK_QUOTES_APPROVAL_REQUIRED) + fetchSwapsQuoteRefreshTimeStub.resolves(getMockQuoteRefreshTime()) // Ensure approval is required sandbox @@ -735,6 +736,7 @@ describe('SwapsController', function () { it('marks the best quote', async function () { fetchTradesInfoStub.resolves(getMockQuotes()) + fetchSwapsQuoteRefreshTimeStub.resolves(getMockQuoteRefreshTime()) // Make it so approval is not required sandbox @@ -765,6 +767,7 @@ describe('SwapsController', function () { } const quotes = { ...getMockQuotes(), [bestAggId]: bestQuote } fetchTradesInfoStub.resolves(quotes) + fetchSwapsQuoteRefreshTimeStub.resolves(getMockQuoteRefreshTime()) // Make it so approval is not required sandbox @@ -782,6 +785,7 @@ describe('SwapsController', function () { it('does not mark as best quote if no conversion rate exists for destination token', async function () { fetchTradesInfoStub.resolves(getMockQuotes()) + fetchSwapsQuoteRefreshTimeStub.resolves(getMockQuoteRefreshTime()) // Make it so approval is not required sandbox @@ -1626,3 +1630,9 @@ function getTopQuoteAndSavingsBaseExpectedResults() { }, } } + +function getMockQuoteRefreshTime() { + return { + seconds: 45 + } +} \ No newline at end of file From 0a0de8c6fa0aa67102d6e673cacf3fc6b4867a2f Mon Sep 17 00:00:00 2001 From: David Walsh Date: Tue, 15 Dec 2020 10:19:09 -0600 Subject: [PATCH 07/10] Lint --- app/scripts/controllers/swaps.js | 10 ++-------- test/unit/app/controllers/swaps-test.js | 6 +++--- 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/app/scripts/controllers/swaps.js b/app/scripts/controllers/swaps.js index e16957207a89..236b10376d09 100644 --- a/app/scripts/controllers/swaps.js +++ b/app/scripts/controllers/swaps.js @@ -130,10 +130,8 @@ export default class SwapsController { }) console.error('Request for swaps quote refresh time failed: ', e) } - - const { - swapsState - } = this.store.getState() + + const { swapsState } = this.store.getState() this.store.updateState({ swapsState: { ...swapsState, swapsQuoteRefreshTime }, }) @@ -457,10 +455,6 @@ export default class SwapsController { }) } - setSwapsQuoteRefreshTime(swapsQuoteRefreshTime) { - - } - resetPostFetchState() { const { swapsState } = this.store.getState() diff --git a/test/unit/app/controllers/swaps-test.js b/test/unit/app/controllers/swaps-test.js index 3830be6daec9..ac38784e0385 100644 --- a/test/unit/app/controllers/swaps-test.js +++ b/test/unit/app/controllers/swaps-test.js @@ -650,7 +650,7 @@ describe('SwapsController', function () { sandbox .stub(swapsController, '_getERC20Allowance') .resolves(ethers.BigNumber.from(1)) - + const [newQuotes] = await swapsController.fetchAndSetQuotes( MOCK_FETCH_PARAMS, MOCK_FETCH_METADATA, @@ -1633,6 +1633,6 @@ function getTopQuoteAndSavingsBaseExpectedResults() { function getMockQuoteRefreshTime() { return { - seconds: 45 + seconds: 45, } -} \ No newline at end of file +} From bb9b1be0064f93607497a9ed217b8e8c69a4c593 Mon Sep 17 00:00:00 2001 From: David Walsh Date: Tue, 15 Dec 2020 11:51:19 -0600 Subject: [PATCH 08/10] Return proper timeout in tests --- test/unit/app/controllers/swaps-test.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/unit/app/controllers/swaps-test.js b/test/unit/app/controllers/swaps-test.js index ac38784e0385..26b6de78a00e 100644 --- a/test/unit/app/controllers/swaps-test.js +++ b/test/unit/app/controllers/swaps-test.js @@ -1632,7 +1632,5 @@ function getTopQuoteAndSavingsBaseExpectedResults() { } function getMockQuoteRefreshTime() { - return { - seconds: 45, - } + return 45000 } From 8c7d2a556d36838e6886e80bc1029446268eece7 Mon Sep 17 00:00:00 2001 From: David Walsh Date: Tue, 15 Dec 2020 12:29:57 -0600 Subject: [PATCH 09/10] Remove event tracking --- app/scripts/controllers/swaps.js | 9 --------- app/scripts/metamask-controller.js | 3 --- 2 files changed, 12 deletions(-) diff --git a/app/scripts/controllers/swaps.js b/app/scripts/controllers/swaps.js index 236b10376d09..647b6b5534eb 100644 --- a/app/scripts/controllers/swaps.js +++ b/app/scripts/controllers/swaps.js @@ -78,7 +78,6 @@ export default class SwapsController { provider, getProviderConfig, tokenRatesStore, - trackMetaMetricsEvent, fetchTradesInfo = defaultFetchTradesInfo, fetchSwapsFeatureLiveness = defaultFetchSwapsFeatureLiveness, fetchSwapsQuoteRefreshTime = defaultFetchSwapsQuoteRefreshTime, @@ -90,7 +89,6 @@ export default class SwapsController { this._fetchTradesInfo = fetchTradesInfo this._fetchSwapsFeatureLiveness = fetchSwapsFeatureLiveness this._fetchSwapsQuoteRefreshTime = fetchSwapsQuoteRefreshTime - this._trackEvent = trackMetaMetricsEvent this.getBufferedGasLimit = getBufferedGasLimit this.tokenRatesStore = tokenRatesStore @@ -121,13 +119,6 @@ export default class SwapsController { FALLBACK_QUOTE_REFRESH_TIME, ) } catch (e) { - this._trackEvent({ - category: 'Swaps', - event: 'quote_refresh_api_error', - properties: { - error_message: e.message, - }, - }) console.error('Request for swaps quote refresh time failed: ', e) } diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 92d01caf820a..f86bb14681ec 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -352,9 +352,6 @@ export default class MetamaskController extends EventEmitter { this.networkController, ), tokenRatesStore: this.tokenRatesController.store, - trackMetaMetricsEvent: this.metaMetricsController.trackEvent.bind( - this.metaMetricsController, - ), }) // ensure isClientOpenAndUnlocked is updated when memState updates From 24dd8fe4b4711de3be24e3360cdc5eb9f155f452 Mon Sep 17 00:00:00 2001 From: David Walsh Date: Tue, 15 Dec 2020 13:05:40 -0600 Subject: [PATCH 10/10] Validate refresh response, don't provide defaultValue --- app/scripts/controllers/swaps.js | 6 ++---- test/unit/app/controllers/swaps-test.js | 2 +- ui/app/ducks/swaps/swaps.js | 1 + ui/app/helpers/utils/fetch-with-cache.js | 1 - ui/app/pages/swaps/swaps.util.js | 8 +++++--- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/app/scripts/controllers/swaps.js b/app/scripts/controllers/swaps.js index 647b6b5534eb..e230b8d6329d 100644 --- a/app/scripts/controllers/swaps.js +++ b/app/scripts/controllers/swaps.js @@ -67,7 +67,7 @@ const initialState = { topAggId: null, routeState: '', swapsFeatureIsLive: false, - swapsQuoteRefreshTime: 0, + swapsQuoteRefreshTime: FALLBACK_QUOTE_REFRESH_TIME, }, } @@ -115,9 +115,7 @@ export default class SwapsController { // Default to fallback time unless API returns valid response let swapsQuoteRefreshTime = FALLBACK_QUOTE_REFRESH_TIME try { - swapsQuoteRefreshTime = await this._fetchSwapsQuoteRefreshTime( - FALLBACK_QUOTE_REFRESH_TIME, - ) + swapsQuoteRefreshTime = await this._fetchSwapsQuoteRefreshTime() } catch (e) { console.error('Request for swaps quote refresh time failed: ', e) } diff --git a/test/unit/app/controllers/swaps-test.js b/test/unit/app/controllers/swaps-test.js index 26b6de78a00e..6aa3e047629e 100644 --- a/test/unit/app/controllers/swaps-test.js +++ b/test/unit/app/controllers/swaps-test.js @@ -121,7 +121,7 @@ const EMPTY_INIT_STATE = { topAggId: null, routeState: '', swapsFeatureIsLive: false, - swapsQuoteRefreshTime: 0, + swapsQuoteRefreshTime: 60000, }, } diff --git a/ui/app/ducks/swaps/swaps.js b/ui/app/ducks/swaps/swaps.js index 48ee7fe5ef70..f6d36c20403a 100644 --- a/ui/app/ducks/swaps/swaps.js +++ b/ui/app/ducks/swaps/swaps.js @@ -330,6 +330,7 @@ export const navigateBackToBuildQuote = (history) => { return async (dispatch) => { // TODO: Ensure any fetch in progress is cancelled dispatch(navigatedBackToBuildQuote()) + history.push(BUILD_QUOTE_ROUTE) } } diff --git a/ui/app/helpers/utils/fetch-with-cache.js b/ui/app/helpers/utils/fetch-with-cache.js index faabc2cbe9a2..351ee4ab9497 100644 --- a/ui/app/helpers/utils/fetch-with-cache.js +++ b/ui/app/helpers/utils/fetch-with-cache.js @@ -48,7 +48,6 @@ const fetchWithCache = async ( cachedResponse: responseJson, cachedTime: currentTime, } - cachedFetch[url] = cacheEntry await setStorageItem('cachedFetch', cachedFetch) return responseJson diff --git a/ui/app/pages/swaps/swaps.util.js b/ui/app/pages/swaps/swaps.util.js index 8fdf45b2d941..81876d947307 100644 --- a/ui/app/pages/swaps/swaps.util.js +++ b/ui/app/pages/swaps/swaps.util.js @@ -332,7 +332,7 @@ export async function fetchSwapsFeatureLiveness() { return status?.active } -export async function fetchSwapsQuoteRefreshTime(defaultValue = 0) { +export async function fetchSwapsQuoteRefreshTime() { const response = await fetchWithCache( getBaseApi('refreshTime'), { method: 'GET' }, @@ -340,11 +340,13 @@ export async function fetchSwapsQuoteRefreshTime(defaultValue = 0) { ) // We presently use milliseconds in the UI - if (response?.seconds) { + if (typeof response?.seconds === 'number' && response.seconds > 0) { return response.seconds * 1000 } - return defaultValue + throw new Error( + `MetaMask - refreshTime provided invalid response: ${response}`, + ) } export async function fetchTokenPrice(address) {