From 28fc2d471fc71d7c152d7a104f0975ca86513431 Mon Sep 17 00:00:00 2001 From: Daniel <80175477+dan437@users.noreply.github.com> Date: Wed, 15 Sep 2021 15:13:18 +0200 Subject: [PATCH] Quotes prefetching in Swaps (#11915) --- app/_locales/en/messages.json | 10 +- app/_locales/es/messages.json | 7 -- app/_locales/es_419/messages.json | 7 -- app/_locales/hi/messages.json | 7 -- app/_locales/id/messages.json | 7 -- app/_locales/it/messages.json | 7 -- app/_locales/ja/messages.json | 7 -- app/_locales/ko/messages.json | 7 -- app/_locales/ph/messages.json | 7 -- app/_locales/pt_BR/messages.json | 7 -- app/_locales/ru/messages.json | 7 -- app/_locales/tl/messages.json | 7 -- app/_locales/vi/messages.json | 7 -- app/_locales/zh_CN/messages.json | 7 -- app/scripts/controllers/swaps.js | 101 ++++++++++++---- app/scripts/controllers/swaps.test.js | 19 ++- app/scripts/metamask-controller.js | 8 ++ test/jest/mock-store.js | 1 + ui/ducks/swaps/swaps.js | 21 +++- ui/pages/swaps/awaiting-swap/awaiting-swap.js | 10 +- ui/pages/swaps/build-quote/build-quote.js | 108 +++++++++++++++--- .../swaps/build-quote/build-quote.test.js | 31 ++++- ui/pages/swaps/index.js | 10 +- .../aggregator-logo.test.js.snap | 18 --- .../loading-swaps-quotes/aggregator-logo.js | 40 ------- .../aggregator-logo.test.js | 22 ---- .../loading-swaps-quotes.js | 93 +-------------- .../loading-swaps-quotes.test.js | 1 - ui/pages/swaps/swaps.util.js | 25 +--- ui/pages/swaps/view-quote/view-quote.js | 38 ++++-- ui/pages/swaps/view-quote/view-quote.test.js | 1 + ui/store/actions.js | 16 +++ 32 files changed, 302 insertions(+), 362 deletions(-) delete mode 100644 ui/pages/swaps/loading-swaps-quotes/__snapshots__/aggregator-logo.test.js.snap delete mode 100644 ui/pages/swaps/loading-swaps-quotes/aggregator-logo.js delete mode 100644 ui/pages/swaps/loading-swaps-quotes/aggregator-logo.test.js diff --git a/app/_locales/en/messages.json b/app/_locales/en/messages.json index 948967e4e..31a10e9f1 100644 --- a/app/_locales/en/messages.json +++ b/app/_locales/en/messages.json @@ -2155,10 +2155,6 @@ "message": "No tokens available matching $1", "description": "Tells the user that a given search string does not match any tokens in our token lists. $1 can be any string of text" }, - "swapCheckingQuote": { - "message": "Checking $1", - "description": "Shown to the user during quote loading. $1 is the name of an aggregator. The message indicates that metamask is currently checking if that aggregator has a trade/quote for their requested swap." - }, "swapConfirmWithHwWallet": { "message": "Confirm with your hardware wallet" }, @@ -2204,6 +2200,9 @@ "swapFailedErrorTitle": { "message": "Swap failed" }, + "swapFetchingQuotes": { + "message": "Fetching quotes" + }, "swapFetchingQuotesErrorDescription": { "message": "Hmmm... something went wrong. Try again, or if errors persist, contact customer support." }, @@ -2213,9 +2212,6 @@ "swapFetchingTokens": { "message": "Fetching tokens..." }, - "swapFinalizing": { - "message": "Finalizing..." - }, "swapFromTo": { "message": "The swap of $1 to $2", "description": "Tells a user that they need to confirm on their hardware wallet a swap of 2 tokens. $1 is a source token and $2 is a destination token" diff --git a/app/_locales/es/messages.json b/app/_locales/es/messages.json index 6e49c2494..2334f927e 100644 --- a/app/_locales/es/messages.json +++ b/app/_locales/es/messages.json @@ -1870,10 +1870,6 @@ "message": "No hay tokens disponibles que coincidan con $1", "description": "Tells the user that a given search string does not match any tokens in our token lists. $1 can be any string of text" }, - "swapCheckingQuote": { - "message": "Comprobando $1", - "description": "Shown to the user during quote loading. $1 is the name of an aggregator. The message indicates that metamask is currently checking if that aggregator has a trade/quote for their requested swap." - }, "swapConfirmWithHwWallet": { "message": "Confirmar con la cartera de hardware" }, @@ -1925,9 +1921,6 @@ "swapFetchingTokens": { "message": "Capturando tokens…" }, - "swapFinalizing": { - "message": "Finalizando…" - }, "swapFromTo": { "message": "El canje de $1 por $2", "description": "Tells a user that they need to confirm on their hardware wallet a swap of 2 tokens. $1 is a source token and $2 is a destination token" diff --git a/app/_locales/es_419/messages.json b/app/_locales/es_419/messages.json index 6e49c2494..2334f927e 100644 --- a/app/_locales/es_419/messages.json +++ b/app/_locales/es_419/messages.json @@ -1870,10 +1870,6 @@ "message": "No hay tokens disponibles que coincidan con $1", "description": "Tells the user that a given search string does not match any tokens in our token lists. $1 can be any string of text" }, - "swapCheckingQuote": { - "message": "Comprobando $1", - "description": "Shown to the user during quote loading. $1 is the name of an aggregator. The message indicates that metamask is currently checking if that aggregator has a trade/quote for their requested swap." - }, "swapConfirmWithHwWallet": { "message": "Confirmar con la cartera de hardware" }, @@ -1925,9 +1921,6 @@ "swapFetchingTokens": { "message": "Capturando tokens…" }, - "swapFinalizing": { - "message": "Finalizando…" - }, "swapFromTo": { "message": "El canje de $1 por $2", "description": "Tells a user that they need to confirm on their hardware wallet a swap of 2 tokens. $1 is a source token and $2 is a destination token" diff --git a/app/_locales/hi/messages.json b/app/_locales/hi/messages.json index 73f2e0dbf..322e798cb 100644 --- a/app/_locales/hi/messages.json +++ b/app/_locales/hi/messages.json @@ -1870,10 +1870,6 @@ "message": "$1 के मिलान वाले कोई भी टोकन उपलब्ध नहीं हैं", "description": "Tells the user that a given search string does not match any tokens in our token lists. $1 can be any string of text" }, - "swapCheckingQuote": { - "message": "$1 की जाँच की जा रही है", - "description": "Shown to the user during quote loading. $1 is the name of an aggregator. The message indicates that metamask is currently checking if that aggregator has a trade/quote for their requested swap." - }, "swapConfirmWithHwWallet": { "message": "अपने हार्डवेयर वॉलेट से पुष्टि करें" }, @@ -1925,9 +1921,6 @@ "swapFetchingTokens": { "message": "टोकन प्राप्त किए जा रहे हैं..." }, - "swapFinalizing": { - "message": "अंतिम रूप दिया जा रहा है..." - }, "swapFromTo": { "message": "$1 से $2 का स्वैप", "description": "Tells a user that they need to confirm on their hardware wallet a swap of 2 tokens. $1 is a source token and $2 is a destination token" diff --git a/app/_locales/id/messages.json b/app/_locales/id/messages.json index bd6d4b3f9..dc3ff2a7f 100644 --- a/app/_locales/id/messages.json +++ b/app/_locales/id/messages.json @@ -1870,10 +1870,6 @@ "message": "Tidak ada token yang cocok yang tersedia $1", "description": "Tells the user that a given search string does not match any tokens in our token lists. $1 can be any string of text" }, - "swapCheckingQuote": { - "message": "Memeriksa $1", - "description": "Shown to the user during quote loading. $1 is the name of an aggregator. The message indicates that metamask is currently checking if that aggregator has a trade/quote for their requested swap." - }, "swapConfirmWithHwWallet": { "message": "Konfirmasikan dengan dompet perangkat keras Anda" }, @@ -1925,9 +1921,6 @@ "swapFetchingTokens": { "message": "Mengambil token..." }, - "swapFinalizing": { - "message": "Menyelesaikan..." - }, "swapFromTo": { "message": "Penukaran dari $1 ke $2", "description": "Tells a user that they need to confirm on their hardware wallet a swap of 2 tokens. $1 is a source token and $2 is a destination token" diff --git a/app/_locales/it/messages.json b/app/_locales/it/messages.json index d947c64d6..88bee983f 100644 --- a/app/_locales/it/messages.json +++ b/app/_locales/it/messages.json @@ -1522,10 +1522,6 @@ "message": "Non ci sono token disponibile con questo nome $1", "description": "Tells the user that a given search string does not match any tokens in our token lists. $1 can be any string of text" }, - "swapCheckingQuote": { - "message": "Verificando $1", - "description": "Shown to the user during quote loading. $1 is the name of an aggregator. The message indicates that metamask is currently checking if that aggregator has a trade/quote for their requested swap." - }, "swapCustom": { "message": "personalizza" }, @@ -1564,9 +1560,6 @@ "swapFetchingTokens": { "message": "Recuperando i token..." }, - "swapFinalizing": { - "message": "Finalizzando..." - }, "swapLowSlippageError": { "message": "La transazione può fallire, il massimo slippage è troppo basso." }, diff --git a/app/_locales/ja/messages.json b/app/_locales/ja/messages.json index 532f807e9..c567f7eff 100644 --- a/app/_locales/ja/messages.json +++ b/app/_locales/ja/messages.json @@ -1870,10 +1870,6 @@ "message": "$1 と一致するトークンがありません", "description": "Tells the user that a given search string does not match any tokens in our token lists. $1 can be any string of text" }, - "swapCheckingQuote": { - "message": "$1 をチェック中", - "description": "Shown to the user during quote loading. $1 is the name of an aggregator. The message indicates that metamask is currently checking if that aggregator has a trade/quote for their requested swap." - }, "swapConfirmWithHwWallet": { "message": "ハードウェア ウォレットで確認する" }, @@ -1925,9 +1921,6 @@ "swapFetchingTokens": { "message": "トークンを取り出し中..." }, - "swapFinalizing": { - "message": "終了中..." - }, "swapFromTo": { "message": "$1 から $2 のスワップ", "description": "Tells a user that they need to confirm on their hardware wallet a swap of 2 tokens. $1 is a source token and $2 is a destination token" diff --git a/app/_locales/ko/messages.json b/app/_locales/ko/messages.json index 916a79da2..10b9cfab7 100644 --- a/app/_locales/ko/messages.json +++ b/app/_locales/ko/messages.json @@ -1870,10 +1870,6 @@ "message": "$1와(과) 일치하는 토큰이 없습니다.", "description": "Tells the user that a given search string does not match any tokens in our token lists. $1 can be any string of text" }, - "swapCheckingQuote": { - "message": "$1 확인 중", - "description": "Shown to the user during quote loading. $1 is the name of an aggregator. The message indicates that metamask is currently checking if that aggregator has a trade/quote for their requested swap." - }, "swapConfirmWithHwWallet": { "message": "하드웨어 지갑으로 확인합니다." }, @@ -1925,9 +1921,6 @@ "swapFetchingTokens": { "message": "토큰 가져오는 중..." }, - "swapFinalizing": { - "message": "마무리 중..." - }, "swapFromTo": { "message": "$1을(를) $2(으)로 스왑", "description": "Tells a user that they need to confirm on their hardware wallet a swap of 2 tokens. $1 is a source token and $2 is a destination token" diff --git a/app/_locales/ph/messages.json b/app/_locales/ph/messages.json index c2fc07073..280e98d5e 100644 --- a/app/_locales/ph/messages.json +++ b/app/_locales/ph/messages.json @@ -1870,10 +1870,6 @@ "message": "Walang available na token na tumutugma sa $1", "description": "Tells the user that a given search string does not match any tokens in our token lists. $1 can be any string of text" }, - "swapCheckingQuote": { - "message": "Sinusuri ang $1", - "description": "Shown to the user during quote loading. $1 is the name of an aggregator. The message indicates that metamask is currently checking if that aggregator has a trade/quote for their requested swap." - }, "swapConfirmWithHwWallet": { "message": "Kumpirmahin ang iyong hardware wallet" }, @@ -1925,9 +1921,6 @@ "swapFetchingTokens": { "message": "Kinukuha ang mga token..." }, - "swapFinalizing": { - "message": "Isinasapinal..." - }, "swapFromTo": { "message": "Ang pag-swap ng $1 sa $2", "description": "Tells a user that they need to confirm on their hardware wallet a swap of 2 tokens. $1 is a source token and $2 is a destination token" diff --git a/app/_locales/pt_BR/messages.json b/app/_locales/pt_BR/messages.json index c9953baac..90ae89a9c 100644 --- a/app/_locales/pt_BR/messages.json +++ b/app/_locales/pt_BR/messages.json @@ -1870,10 +1870,6 @@ "message": "Nenhum token disponível correspondente a $1", "description": "Tells the user that a given search string does not match any tokens in our token lists. $1 can be any string of text" }, - "swapCheckingQuote": { - "message": "Verificando $1", - "description": "Shown to the user during quote loading. $1 is the name of an aggregator. The message indicates that metamask is currently checking if that aggregator has a trade/quote for their requested swap." - }, "swapConfirmWithHwWallet": { "message": "Confirme com sua carteira de hardware" }, @@ -1925,9 +1921,6 @@ "swapFetchingTokens": { "message": "Fetch dos tokens..." }, - "swapFinalizing": { - "message": "Finalizando..." - }, "swapFromTo": { "message": "O swap de $1 para $2", "description": "Tells a user that they need to confirm on their hardware wallet a swap of 2 tokens. $1 is a source token and $2 is a destination token" diff --git a/app/_locales/ru/messages.json b/app/_locales/ru/messages.json index ddf3f9b72..2cc556eb6 100644 --- a/app/_locales/ru/messages.json +++ b/app/_locales/ru/messages.json @@ -1870,10 +1870,6 @@ "message": "Нет доступных токенов соответствующих $1", "description": "Tells the user that a given search string does not match any tokens in our token lists. $1 can be any string of text" }, - "swapCheckingQuote": { - "message": "Проверка $1", - "description": "Shown to the user during quote loading. $1 is the name of an aggregator. The message indicates that metamask is currently checking if that aggregator has a trade/quote for their requested swap." - }, "swapConfirmWithHwWallet": { "message": "Подтвердить с помощью аппаратного кошелька" }, @@ -1925,9 +1921,6 @@ "swapFetchingTokens": { "message": "Получение токенов..." }, - "swapFinalizing": { - "message": "Завершение..." - }, "swapFromTo": { "message": "Своп $1 на $2", "description": "Tells a user that they need to confirm on their hardware wallet a swap of 2 tokens. $1 is a source token and $2 is a destination token" diff --git a/app/_locales/tl/messages.json b/app/_locales/tl/messages.json index 4de586d30..02280e381 100644 --- a/app/_locales/tl/messages.json +++ b/app/_locales/tl/messages.json @@ -1510,10 +1510,6 @@ "message": "Walang available na token na tumutugma sa $1", "description": "Tells the user that a given search string does not match any tokens in our token lists. $1 can be any string of text" }, - "swapCheckingQuote": { - "message": "Sinusuri ang $1", - "description": "Shown to the user during quote loading. $1 is the name of an aggregator. The message indicates that metamask is currently checking if that aggregator has a trade/quote for their requested swap." - }, "swapCustom": { "message": "custom" }, @@ -1552,9 +1548,6 @@ "swapFetchingTokens": { "message": "Kinukuha ang mga token..." }, - "swapFinalizing": { - "message": "Isinasapinal..." - }, "swapLowSlippageError": { "message": "Maaaring hindi magtagumpay ang transaksyon, masyadong mababa ang max na slippage." }, diff --git a/app/_locales/vi/messages.json b/app/_locales/vi/messages.json index 565818338..b28cbf86a 100644 --- a/app/_locales/vi/messages.json +++ b/app/_locales/vi/messages.json @@ -1870,10 +1870,6 @@ "message": "Không có token nào khớp với $1", "description": "Tells the user that a given search string does not match any tokens in our token lists. $1 can be any string of text" }, - "swapCheckingQuote": { - "message": "Đang kiểm tra $1", - "description": "Shown to the user during quote loading. $1 is the name of an aggregator. The message indicates that metamask is currently checking if that aggregator has a trade/quote for their requested swap." - }, "swapConfirmWithHwWallet": { "message": "Xác nhận ví cứng của bạn" }, @@ -1925,9 +1921,6 @@ "swapFetchingTokens": { "message": "Đang tìm nạp token..." }, - "swapFinalizing": { - "message": "Đang hoàn tất..." - }, "swapFromTo": { "message": "Giao dịch hoán đổi $1 sang $2", "description": "Tells a user that they need to confirm on their hardware wallet a swap of 2 tokens. $1 is a source token and $2 is a destination token" diff --git a/app/_locales/zh_CN/messages.json b/app/_locales/zh_CN/messages.json index 3427cec44..02ec5f565 100644 --- a/app/_locales/zh_CN/messages.json +++ b/app/_locales/zh_CN/messages.json @@ -1516,10 +1516,6 @@ "message": "没有匹配的代币符合 $1", "description": "Tells the user that a given search string does not match any tokens in our token lists. $1 can be any string of text" }, - "swapCheckingQuote": { - "message": "正在检查 $1", - "description": "Shown to the user during quote loading. $1 is the name of an aggregator. The message indicates that metamask is currently checking if that aggregator has a trade/quote for their requested swap." - }, "swapCustom": { "message": "自定义" }, @@ -1558,9 +1554,6 @@ "swapFetchingTokens": { "message": "获取代币中……" }, - "swapFinalizing": { - "message": "确定中……" - }, "swapLowSlippageError": { "message": "交易可能失败,最大滑点过低。" }, diff --git a/app/scripts/controllers/swaps.js b/app/scripts/controllers/swaps.js index 238dba04c..dea26345b 100644 --- a/app/scripts/controllers/swaps.js +++ b/app/scripts/controllers/swaps.js @@ -24,8 +24,9 @@ import { isSwapsDefaultTokenAddress } from '../../../shared/modules/swaps.utils' import { fetchTradesInfo as defaultFetchTradesInfo, - fetchSwapsQuoteRefreshTime as defaultFetchSwapsQuoteRefreshTime, + getBaseApi, } from '../../../ui/pages/swaps/swaps.util'; +import fetchWithCache from '../../../ui/helpers/utils/fetch-with-cache'; import { MINUTE, SECOND } from '../../../shared/constants/time'; import { NETWORK_EVENTS } from './network'; @@ -40,10 +41,6 @@ const POLL_COUNT_LIMIT = 3; // provide a reasonable fallback to avoid further errors const FALLBACK_QUOTE_REFRESH_TIME = MINUTE; -// 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 = SECOND * 10; - function calculateGasEstimateWithRefund( maxGas = MAX_GAS_LIMIT, estimatedRefund = 0, @@ -64,6 +61,7 @@ function calculateGasEstimateWithRefund( const initialState = { swapsState: { quotes: {}, + quotesPollingLimitEnabled: false, fetchParams: null, tokens: null, tradeTxId: null, @@ -82,6 +80,7 @@ const initialState = { swapsFeatureIsLive: true, useNewSwapsApi: false, swapsQuoteRefreshTime: FALLBACK_QUOTE_REFRESH_TIME, + swapsQuotePrefetchingRefreshTime: FALLBACK_QUOTE_REFRESH_TIME, }, }; @@ -93,7 +92,6 @@ export default class SwapsController { getProviderConfig, tokenRatesStore, fetchTradesInfo = defaultFetchTradesInfo, - fetchSwapsQuoteRefreshTime = defaultFetchSwapsQuoteRefreshTime, getCurrentChainId, getEIP1559GasFeeEstimates, }) { @@ -102,7 +100,6 @@ export default class SwapsController { }); this._fetchTradesInfo = fetchTradesInfo; - this._fetchSwapsQuoteRefreshTime = fetchSwapsQuoteRefreshTime; this._getCurrentChainId = getCurrentChainId; this._getEIP1559GasFeeEstimates = getEIP1559GasFeeEstimates; @@ -124,38 +121,70 @@ export default class SwapsController { }); } + async fetchSwapsRefreshRates(chainId, useNewSwapsApi) { + const response = await fetchWithCache( + getBaseApi('network', chainId, useNewSwapsApi), + { method: 'GET' }, + { cacheRefreshTime: 600000 }, + ); + const { refreshRates } = response || {}; + if ( + !refreshRates || + typeof refreshRates.quotes !== 'number' || + typeof refreshRates.quotesPrefetching !== 'number' + ) { + throw new Error( + `MetaMask - invalid response for refreshRates: ${response}`, + ); + } + // We presently use milliseconds in the UI. + return { + quotes: refreshRates.quotes * 1000, + quotesPrefetching: refreshRates.quotesPrefetching * 1000, + }; + } + // Sets the refresh rate for quote updates from the MetaSwap API - async _setSwapsQuoteRefreshTime() { + async _setSwapsRefreshRates() { const chainId = this._getCurrentChainId(); const { swapsState } = this.store.getState(); - - // Default to fallback time unless API returns valid response - let swapsQuoteRefreshTime = FALLBACK_QUOTE_REFRESH_TIME; + let swapsRefreshRates; try { - swapsQuoteRefreshTime = await this._fetchSwapsQuoteRefreshTime( + swapsRefreshRates = await this.fetchSwapsRefreshRates( chainId, swapsState.useNewSwapsApi, ); } catch (e) { console.error('Request for swaps quote refresh time failed: ', e); } - const { swapsState: latestSwapsState } = this.store.getState(); - this.store.updateState({ - swapsState: { ...latestSwapsState, swapsQuoteRefreshTime }, + swapsState: { + ...latestSwapsState, + swapsQuoteRefreshTime: + swapsRefreshRates?.quotes || FALLBACK_QUOTE_REFRESH_TIME, + swapsQuotePrefetchingRefreshTime: + swapsRefreshRates?.quotesPrefetching || FALLBACK_QUOTE_REFRESH_TIME, + }, }); } // 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 - // that quotes will no longer be available after 1 or 2 minutes. When fetchAndSetQuotes is first called it, receives fetch that parameters are stored in + // that quotes will no longer be available after 1 or 2 minutes. When fetchAndSetQuotes is first called, it receives fetch parameters that are stored in // 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 }, + swapsState: { + swapsQuoteRefreshTime, + swapsQuotePrefetchingRefreshTime, + quotesPollingLimitEnabled, + }, } = this.store.getState(); - + // swapsQuoteRefreshTime is used on the View Quote page, swapsQuotePrefetchingRefreshTime is used on the Build Quote page. + const quotesRefreshRateInMs = quotesPollingLimitEnabled + ? swapsQuoteRefreshTime + : swapsQuotePrefetchingRefreshTime; this.pollingTimeout = setTimeout(() => { const { swapsState } = this.store.getState(); this.fetchAndSetQuotes( @@ -163,11 +192,13 @@ export default class SwapsController { swapsState.fetchParams?.metaData, true, ); - }, swapsQuoteRefreshTime - QUOTE_POLLING_DIFFERENCE_INTERVAL); + }, quotesRefreshRateInMs); } stopPollingForQuotes() { - clearTimeout(this.pollingTimeout); + if (this.pollingTimeout) { + clearTimeout(this.pollingTimeout); + } } async fetchAndSetQuotes( @@ -177,7 +208,7 @@ export default class SwapsController { ) { const { chainId } = fetchParamsMetaData; const { - swapsState: { useNewSwapsApi }, + swapsState: { useNewSwapsApi, quotesPollingLimitEnabled }, } = this.store.getState(); if (!fetchParams) { @@ -203,7 +234,7 @@ export default class SwapsController { ...fetchParamsMetaData, useNewSwapsApi, }), - this._setSwapsQuoteRefreshTime(), + this._setSwapsRefreshRates(), ]); newQuotes = mapValues(newQuotes, (quote) => ({ @@ -292,9 +323,13 @@ export default class SwapsController { }, }); - // We only want to do up to a maximum of three requests from polling. - this.pollCount += 1; - if (this.pollCount < POLL_COUNT_LIMIT + 1) { + if (quotesPollingLimitEnabled) { + // We only want to do up to a maximum of three requests from polling if polling limit is enabled. + // Otherwise we won't increase pollCount, so polling will run without a limit. + this.pollCount += 1; + } + + if (!quotesPollingLimitEnabled || this.pollCount < POLL_COUNT_LIMIT + 1) { this.pollForNewQuotes(); } else { this.resetPostFetchState(); @@ -322,6 +357,11 @@ export default class SwapsController { this.store.updateState({ swapsState: { ...swapsState, tokens } }); } + clearSwapsQuotes() { + const { swapsState } = this.store.getState(); + this.store.updateState({ swapsState: { ...swapsState, quotes: {} } }); + } + setSwapsErrorKey(errorKey) { const { swapsState } = this.store.getState(); this.store.updateState({ swapsState: { ...swapsState, errorKey } }); @@ -464,6 +504,13 @@ export default class SwapsController { }); } + setSwapsQuotesPollingLimitEnabled(quotesPollingLimitEnabled) { + const { swapsState } = this.store.getState(); + this.store.updateState({ + swapsState: { ...swapsState, quotesPollingLimitEnabled }, + }); + } + setSwapsTxMaxFeePriorityPerGas(maxPriorityFeePerGas) { const { swapsState } = this.store.getState(); this.store.updateState({ @@ -511,6 +558,8 @@ export default class SwapsController { swapsFeatureIsLive: swapsState.swapsFeatureIsLive, useNewSwapsApi: swapsState.useNewSwapsApi, swapsQuoteRefreshTime: swapsState.swapsQuoteRefreshTime, + swapsQuotePrefetchingRefreshTime: + swapsState.swapsQuotePrefetchingRefreshTime, }, }); clearTimeout(this.pollingTimeout); @@ -523,6 +572,8 @@ export default class SwapsController { ...initialState.swapsState, tokens: swapsState.tokens, swapsQuoteRefreshTime: swapsState.swapsQuoteRefreshTime, + swapsQuotePrefetchingRefreshTime: + swapsState.swapsQuotePrefetchingRefreshTime, }, }); clearTimeout(this.pollingTimeout); diff --git a/app/scripts/controllers/swaps.test.js b/app/scripts/controllers/swaps.test.js index 4ed83ef25..9264ab8b2 100644 --- a/app/scripts/controllers/swaps.test.js +++ b/app/scripts/controllers/swaps.test.js @@ -116,6 +116,7 @@ function getMockNetworkController() { const EMPTY_INIT_STATE = { swapsState: { quotes: {}, + quotesPollingLimitEnabled: false, fetchParams: null, tokens: null, tradeTxId: null, @@ -133,13 +134,13 @@ const EMPTY_INIT_STATE = { swapsFeatureIsLive: true, useNewSwapsApi: false, swapsQuoteRefreshTime: 60000, + swapsQuotePrefetchingRefreshTime: 60000, swapsUserFeeLevel: '', }, }; const sandbox = sinon.createSandbox(); const fetchTradesInfoStub = sandbox.stub(); -const fetchSwapsQuoteRefreshTimeStub = sandbox.stub(); const getCurrentChainIdStub = sandbox.stub(); getCurrentChainIdStub.returns(MAINNET_CHAIN_ID); const getEIP1559GasFeeEstimatesStub = sandbox.stub(() => { @@ -162,7 +163,6 @@ describe('SwapsController', function () { getProviderConfig: MOCK_GET_PROVIDER_CONFIG, tokenRatesStore: MOCK_TOKEN_RATES_STORE, fetchTradesInfo: fetchTradesInfoStub, - fetchSwapsQuoteRefreshTime: fetchSwapsQuoteRefreshTimeStub, getCurrentChainId: getCurrentChainIdStub, getEIP1559GasFeeEstimates: getEIP1559GasFeeEstimatesStub, }); @@ -670,7 +670,6 @@ describe('SwapsController', function () { 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 @@ -716,7 +715,6 @@ describe('SwapsController', function () { it('performs the allowance check', async function () { fetchTradesInfoStub.resolves(getMockQuotes()); - fetchSwapsQuoteRefreshTimeStub.resolves(getMockQuoteRefreshTime()); // Make it so approval is not required const allowanceStub = sandbox @@ -740,7 +738,6 @@ 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 @@ -766,7 +763,6 @@ describe('SwapsController', function () { it('marks the best quote', async function () { fetchTradesInfoStub.resolves(getMockQuotes()); - fetchSwapsQuoteRefreshTimeStub.resolves(getMockQuoteRefreshTime()); // Make it so approval is not required sandbox @@ -797,7 +793,6 @@ describe('SwapsController', function () { }; const quotes = { ...getMockQuotes(), [bestAggId]: bestQuote }; fetchTradesInfoStub.resolves(quotes); - fetchSwapsQuoteRefreshTimeStub.resolves(getMockQuoteRefreshTime()); // Make it so approval is not required sandbox @@ -815,7 +810,6 @@ 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 @@ -843,6 +837,8 @@ describe('SwapsController', function () { ...EMPTY_INIT_STATE.swapsState, tokens: old.tokens, swapsQuoteRefreshTime: old.swapsQuoteRefreshTime, + swapsQuotePrefetchingRefreshTime: + old.swapsQuotePrefetchingRefreshTime, }); }); @@ -890,6 +886,7 @@ describe('SwapsController', function () { const swapsFeatureIsLive = false; const useNewSwapsApi = false; const swapsQuoteRefreshTime = 0; + const swapsQuotePrefetchingRefreshTime = 0; swapsController.store.updateState({ swapsState: { tokens, @@ -897,6 +894,7 @@ describe('SwapsController', function () { swapsFeatureIsLive, useNewSwapsApi, swapsQuoteRefreshTime, + swapsQuotePrefetchingRefreshTime, }, }); @@ -909,6 +907,7 @@ describe('SwapsController', function () { fetchParams, swapsFeatureIsLive, swapsQuoteRefreshTime, + swapsQuotePrefetchingRefreshTime, }); }); }); @@ -1387,7 +1386,3 @@ function getTopQuoteAndSavingsBaseExpectedResults() { }, }; } - -function getMockQuoteRefreshTime() { - return 45000; -} diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 891019324..77e84f530 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -1073,6 +1073,10 @@ export default class MetamaskController extends EventEmitter { swapsController, ), setSwapsTokens: nodeify(swapsController.setSwapsTokens, swapsController), + clearSwapsQuotes: nodeify( + swapsController.clearSwapsQuotes, + swapsController, + ), setApproveTxId: nodeify(swapsController.setApproveTxId, swapsController), setTradeTxId: nodeify(swapsController.setTradeTxId, swapsController), setSwapsTxGasPrice: nodeify( @@ -1127,6 +1131,10 @@ export default class MetamaskController extends EventEmitter { swapsController.setSwapsUserFeeLevel, swapsController, ), + setSwapsQuotesPollingLimitEnabled: nodeify( + swapsController.setSwapsQuotesPollingLimitEnabled, + swapsController, + ), // MetaMetrics trackMetaMetricsEvent: nodeify( diff --git a/test/jest/mock-store.js b/test/jest/mock-store.js index 19dbcf48b..a23d7ad35 100644 --- a/test/jest/mock-store.js +++ b/test/jest/mock-store.js @@ -212,6 +212,7 @@ export const createSwapsMockStore = () => { approveTxId: null, quotesLastFetched: 1519211809934, swapsQuoteRefreshTime: 60000, + swapsQuotePrefetchingRefreshTime: 60000, customMaxGas: '', customGasPrice: null, selectedAggId: 'TEST_AGG_2', diff --git a/ui/ducks/swaps/swaps.js b/ui/ducks/swaps/swaps.js index 5cbb8dedc..4acfcad50 100644 --- a/ui/ducks/swaps/swaps.js +++ b/ui/ducks/swaps/swaps.js @@ -86,6 +86,7 @@ const initialState = { fetchingQuotes: false, fromToken: null, quotesFetchStartTime: null, + reviewSwapClickedTimestamp: null, topAssets: {}, toToken: null, customGas: { @@ -130,6 +131,9 @@ const slice = createSlice({ setQuotesFetchStartTime: (state, action) => { state.quotesFetchStartTime = action.payload; }, + setReviewSwapClickedTimestamp: (state, action) => { + state.reviewSwapClickedTimestamp = action.payload; + }, setTopAssets: (state, action) => { state.topAssets = action.payload; }, @@ -183,6 +187,9 @@ export const getFetchingQuotes = (state) => state.swaps.fetchingQuotes; export const getQuotesFetchStartTime = (state) => state.swaps.quotesFetchStartTime; +export const getReviewSwapClickedTimestamp = (state) => + state.swaps.reviewSwapClickedTimestamp; + export const getSwapsCustomizationModalPrice = (state) => state.swaps.customGas.price; @@ -236,6 +243,9 @@ export const getUseNewSwapsApi = (state) => export const getSwapsQuoteRefreshTime = (state) => state.metamask.swapsState.swapsQuoteRefreshTime; +export const getSwapsQuotePrefetchingRefreshTime = (state) => + state.metamask.swapsState.swapsQuotePrefetchingRefreshTime; + export const getBackgroundSwapRouteState = (state) => state.metamask.swapsState.routeState; @@ -323,6 +333,7 @@ const { setFetchingQuotes, setFromToken, setQuotesFetchStartTime, + setReviewSwapClickedTimestamp, setTopAssets, setToToken, swapCustomGasModalPriceEdited, @@ -338,6 +349,7 @@ export { setFetchingQuotes, setFromToken as setSwapsFromToken, setQuotesFetchStartTime as setSwapQuotesFetchStartTime, + setReviewSwapClickedTimestamp, setTopAssets, setToToken as setSwapToToken, swapCustomGasModalPriceEdited, @@ -412,6 +424,7 @@ export const fetchQuotesAndSetQuoteState = ( inputValue, maxSlippage, metaMetricsEvent, + pageRedirectionDisabled, ) => { return async (dispatch, getState) => { const state = getState(); @@ -461,8 +474,12 @@ export const fetchQuotesAndSetQuoteState = ( decimals: toTokenDecimals, iconUrl: toTokenIconUrl, } = selectedToToken; - await dispatch(setBackgroundSwapRouteState('loading')); - history.push(LOADING_QUOTES_ROUTE); + // pageRedirectionDisabled is true if quotes prefetching is active (a user is on the Build Quote page). + // In that case we just want to silently prefetch quotes without redirecting to the quotes loading page. + if (!pageRedirectionDisabled) { + await dispatch(setBackgroundSwapRouteState('loading')); + history.push(LOADING_QUOTES_ROUTE); + } dispatch(setFetchingQuotes(true)); const contractExchangeRates = getTokenExchangeRates(state); diff --git a/ui/pages/swaps/awaiting-swap/awaiting-swap.js b/ui/pages/swaps/awaiting-swap/awaiting-swap.js index e1a9e934d..d949c5441 100644 --- a/ui/pages/swaps/awaiting-swap/awaiting-swap.js +++ b/ui/pages/swaps/awaiting-swap/awaiting-swap.js @@ -1,5 +1,5 @@ import EventEmitter from 'events'; -import React, { useContext, useRef, useState } from 'react'; +import React, { useContext, useRef, useState, useEffect } from 'react'; import { useDispatch, useSelector } from 'react-redux'; import PropTypes from 'prop-types'; import { useHistory } from 'react-router-dom'; @@ -42,6 +42,7 @@ import { isSwapsDefaultTokenSymbol } from '../../../../shared/modules/swaps.util import PulseLoader from '../../../components/ui/pulse-loader'; import { ASSET_ROUTE, DEFAULT_ROUTE } from '../../../helpers/constants/routes'; +import { stopPollingForQuotes } from '../../../store/actions'; import { getRenderableNetworkFeesForQuote } from '../swaps.util'; import SwapsFooter from '../swaps-footer'; @@ -249,6 +250,13 @@ export default function AwaitingSwap({ ); }; + useEffect(() => { + if (errorKey) { + // If there was an error, stop polling for quotes. + dispatch(stopPollingForQuotes()); + } + }, [dispatch, errorKey]); + return (
diff --git a/ui/pages/swaps/build-quote/build-quote.js b/ui/pages/swaps/build-quote/build-quote.js index 66bfbeb5a..1503da608 100644 --- a/ui/pages/swaps/build-quote/build-quote.js +++ b/ui/pages/swaps/build-quote/build-quote.js @@ -19,6 +19,10 @@ import SlippageButtons from '../slippage-buttons'; import { getTokens, getConversionRate } from '../../../ducks/metamask/metamask'; import InfoTooltip from '../../../components/ui/info-tooltip'; import ActionableMessage from '../../../components/ui/actionable-message/actionable-message'; +import { + VIEW_QUOTE_ROUTE, + LOADING_QUOTES_ROUTE, +} from '../../../helpers/constants/routes'; import { fetchQuotesAndSetQuoteState, @@ -29,6 +33,8 @@ import { getBalanceError, getTopAssets, getFetchParams, + getQuotes, + setReviewSwapClickedTimestamp, } from '../../../ducks/swaps/swaps'; import { getSwapsDefaultToken, @@ -60,7 +66,13 @@ import { SWAPS_CHAINID_DEFAULT_TOKEN_MAP, } from '../../../../shared/constants/swaps'; -import { resetSwapsPostFetchState, removeToken } from '../../../store/actions'; +import { + resetSwapsPostFetchState, + removeToken, + setBackgroundSwapRouteState, + clearSwapsQuotes, + stopPollingForQuotes, +} from '../../../store/actions'; import { fetchTokenPrice, fetchTokenBalance, @@ -76,6 +88,8 @@ const fuseSearchKeys = [ const MAX_ALLOWED_SLIPPAGE = 15; +let timeoutIdForQuotesPrefetching; + export default function BuildQuote({ inputValue, onInputChange, @@ -110,6 +124,8 @@ export default function BuildQuote({ const rpcPrefs = useSelector(getRpcPrefsForCurrentProvider); const tokenList = useSelector(getTokenList); const useTokenDetection = useSelector(getUseTokenDetection); + const quotes = useSelector(getQuotes, isEqual); + const areQuotesPresent = Object.keys(quotes).length > 0; const tokenConversionRates = useSelector(getTokenExchangeRates, isEqual); const conversionRate = useSelector(getConversionRate); @@ -168,6 +184,7 @@ export default function BuildQuote({ decimals: fromTokenDecimals, balance: rawFromTokenBalance, } = selectedFromToken || {}; + const { address: toTokenAddress } = selectedToToken || {}; const fromTokenBalance = rawFromTokenBalance && @@ -348,6 +365,7 @@ export default function BuildQuote({ useEffect(() => { dispatch(resetSwapsPostFetchState()); + dispatch(setReviewSwapClickedTimestamp()); }, [dispatch]); const BlockExplorerLink = () => { @@ -392,6 +410,51 @@ export default function BuildQuote({ fromTokenAddress, selectedToToken.address, ); + const isReviewSwapButtonDisabled = + tokenFromError || + !isFeatureFlagLoaded || + !Number(inputValue) || + !selectedToToken?.address || + Number(maxSlippage) < 0 || + Number(maxSlippage) > MAX_ALLOWED_SLIPPAGE || + (toTokenIsNotDefault && occurrences < 2 && !verificationClicked); + + // It's triggered every time there is a change in form values (token from, token to, amount and slippage). + useEffect(() => { + dispatch(clearSwapsQuotes()); + dispatch(stopPollingForQuotes()); + const prefetchQuotesWithoutRedirecting = async () => { + const pageRedirectionDisabled = true; + await dispatch( + fetchQuotesAndSetQuoteState( + history, + inputValue, + maxSlippage, + metaMetricsEvent, + pageRedirectionDisabled, + ), + ); + }; + // Delay fetching quotes until a user is done typing an input value. If they type a new char in less than a second, + // we will cancel previous setTimeout call and start running a new one. + timeoutIdForQuotesPrefetching = setTimeout(() => { + timeoutIdForQuotesPrefetching = null; + if (!isReviewSwapButtonDisabled) { + // Only do quotes prefetching if the Review Swap button is enabled. + prefetchQuotesWithoutRedirecting(); + } + }, 1000); + return () => clearTimeout(timeoutIdForQuotesPrefetching); + }, [ + dispatch, + history, + maxSlippage, + metaMetricsEvent, + isReviewSwapButtonDisabled, + inputValue, + fromTokenAddress, + toTokenAddress, + ]); return (
@@ -401,6 +464,7 @@ export default function BuildQuote({ {!isSwapsDefaultTokenSymbol(fromTokenSymbol, chainId) && (
onInputChange(fromTokenBalance || '0', fromTokenBalance) } @@ -583,26 +647,32 @@ export default function BuildQuote({ )}
{ - dispatch( - fetchQuotesAndSetQuoteState( - history, - inputValue, - maxSlippage, - metaMetricsEvent, - ), - ); + onSubmit={async () => { + // We need this to know how long it took to go from clicking on the Review Swap button to rendered View Quote page. + dispatch(setReviewSwapClickedTimestamp(Date.now())); + // In case that quotes prefetching is waiting to be executed, but hasn't started yet, + // we want to cancel it and fetch quotes from here. + if (timeoutIdForQuotesPrefetching) { + clearTimeout(timeoutIdForQuotesPrefetching); + dispatch( + fetchQuotesAndSetQuoteState( + history, + inputValue, + maxSlippage, + metaMetricsEvent, + ), + ); + } else if (areQuotesPresent) { + // If there are prefetched quotes already, go directly to the View Quote page. + history.push(VIEW_QUOTE_ROUTE); + } else { + // If the "Review Swap" button was clicked while quotes are being fetched, go to the Loading Quotes page. + await dispatch(setBackgroundSwapRouteState('loading')); + history.push(LOADING_QUOTES_ROUTE); + } }} submitText={t('swapReviewSwap')} - disabled={ - tokenFromError || - !isFeatureFlagLoaded || - !Number(inputValue) || - !selectedToToken?.address || - Number(maxSlippage) < 0 || - Number(maxSlippage) > MAX_ALLOWED_SLIPPAGE || - (toTokenIsNotDefault && occurrences < 2 && !verificationClicked) - } + disabled={isReviewSwapButtonDisabled} hideCancel showTermsOfService /> diff --git a/ui/pages/swaps/build-quote/build-quote.test.js b/ui/pages/swaps/build-quote/build-quote.test.js index 1fc09368e..759ebb9b2 100644 --- a/ui/pages/swaps/build-quote/build-quote.test.js +++ b/ui/pages/swaps/build-quote/build-quote.test.js @@ -1,6 +1,7 @@ import React from 'react'; import configureMockStore from 'redux-mock-store'; import thunk from 'redux-thunk'; +import { fireEvent } from '@testing-library/react'; import { renderWithProvider, @@ -14,7 +15,7 @@ const createProps = (customProps = {}) => { return { inputValue: '5', onInputChange: jest.fn(), - ethBalance: '6 ETH', + ethBalance: '0x8', setMaxSlippage: jest.fn(), maxSlippage: 15, selectedAccountAddress: 'selectedAccountAddress', @@ -26,6 +27,10 @@ const createProps = (customProps = {}) => { setBackgroundConnection({ resetPostFetchState: jest.fn(), + removeToken: jest.fn(), + setBackgroundSwapRouteState: jest.fn(), + clearSwapsQuotes: jest.fn(), + stopPollingForQuotes: jest.fn(), }); describe('BuildQuote', () => { @@ -44,4 +49,28 @@ describe('BuildQuote', () => { document.querySelector('.slippage-buttons__button-group'), ).toMatchSnapshot(); }); + + it('clicks on the max button', () => { + const store = configureMockStore(middleware)(createSwapsMockStore()); + const props = createProps(); + const { getByTestId } = renderWithProvider( + , + store, + ); + fireEvent.click(getByTestId('build-quote__max-button')); + expect(props.onInputChange).toHaveBeenCalled(); + }); + + it('types a number inside the input field', () => { + const store = configureMockStore(middleware)(createSwapsMockStore()); + const props = createProps(); + const { getByDisplayValue } = renderWithProvider( + , + store, + ); + fireEvent.change(getByDisplayValue('5'), { + target: { value: '8' }, + }); + expect(props.onInputChange).toHaveBeenCalled(); + }); }); diff --git a/ui/pages/swaps/index.js b/ui/pages/swaps/index.js index 477469e3c..03772d4df 100644 --- a/ui/pages/swaps/index.js +++ b/ui/pages/swaps/index.js @@ -37,6 +37,7 @@ import { fetchSwapsLiveness, getUseNewSwapsApi, getFromToken, + getReviewSwapClickedTimestamp, } from '../../ducks/swaps/swaps'; import { checkNetworkAndAccountSupports1559, @@ -123,6 +124,8 @@ export default function Swap() { const fromToken = useSelector(getFromToken); const tokenList = useSelector(getTokenList); const listTokenValues = shuffle(Object.values(tokenList)); + const reviewSwapClickedTimestamp = useSelector(getReviewSwapClickedTimestamp); + const reviewSwapClicked = Boolean(reviewSwapClickedTimestamp); if (networkAndAccountSupports1559) { // This will pre-load gas fees before going to the View Quote page. @@ -255,10 +258,12 @@ export default function Swap() { }, [dispatch]); useEffect(() => { - if (swapsErrorKey && !isSwapsErrorRoute) { + // If there is a swapsErrorKey and reviewSwapClicked is false, there was an error in silent quotes prefetching + // and we don't want to show the error page in that case, because another API call for quotes can be successful. + if (swapsErrorKey && !isSwapsErrorRoute && reviewSwapClicked) { history.push(SWAPS_ERROR_ROUTE); } - }, [history, swapsErrorKey, isSwapsErrorRoute]); + }, [history, swapsErrorKey, isSwapsErrorRoute, reviewSwapClicked]); const beforeUnloadEventAddedRef = useRef(); useEffect(() => { @@ -393,7 +398,6 @@ export default function Swap() { } onDone={async () => { await dispatch(setBackgroundSwapRouteState('')); - if ( swapsErrorKey === ERROR_FETCHING_QUOTES || swapsErrorKey === QUOTES_NOT_AVAILABLE_ERROR diff --git a/ui/pages/swaps/loading-swaps-quotes/__snapshots__/aggregator-logo.test.js.snap b/ui/pages/swaps/loading-swaps-quotes/__snapshots__/aggregator-logo.test.js.snap deleted file mode 100644 index f2063ca4d..000000000 --- a/ui/pages/swaps/loading-swaps-quotes/__snapshots__/aggregator-logo.test.js.snap +++ /dev/null @@ -1,18 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`AggregatorLogo renders the component with initial props 1`] = ` -
- -
-`; diff --git a/ui/pages/swaps/loading-swaps-quotes/aggregator-logo.js b/ui/pages/swaps/loading-swaps-quotes/aggregator-logo.js deleted file mode 100644 index 395389ae5..000000000 --- a/ui/pages/swaps/loading-swaps-quotes/aggregator-logo.js +++ /dev/null @@ -1,40 +0,0 @@ -import React from 'react'; -import PropTypes from 'prop-types'; - -// Inspired by https://stackoverflow.com/a/28056903/4727685 -function hexToRGB(hex, alpha) { - const r = parseInt(hex.slice(1, 3), 16); - const g = parseInt(hex.slice(3, 5), 16); - const b = parseInt(hex.slice(5, 7), 16); - - return `rgba(${r}, ${g}, ${b}, ${alpha})`; -} - -export default function AggregatorLogo({ name, icon, color }) { - return ( -
- {icon && color ? ( -
- -
- ) : ( - name && ( -
- {name} -
- ) - )} -
- ); -} - -AggregatorLogo.propTypes = { - name: PropTypes.string, - icon: PropTypes.string, - color: PropTypes.string, -}; diff --git a/ui/pages/swaps/loading-swaps-quotes/aggregator-logo.test.js b/ui/pages/swaps/loading-swaps-quotes/aggregator-logo.test.js deleted file mode 100644 index 3367316b6..000000000 --- a/ui/pages/swaps/loading-swaps-quotes/aggregator-logo.test.js +++ /dev/null @@ -1,22 +0,0 @@ -import React from 'react'; - -import { renderWithProvider } from '../../../../test/jest'; -import AggregatorLogo from './aggregator-logo'; - -const createProps = (customProps = {}) => { - return { - color: '#000', - icon: - 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAASkAAAB5CAYAAABlYNfBAAAACXBIWXMAACxLAAAsSwGlPZapAAAAAXNSR0IArs4c6QAAAARnQU1BAACxjwv8YQUAAAlASURBVHgB7d3/eeO2GcDx1336f3UThNcF4g3KLNA4HaDnywC5Xgeo6Q5Q+zpAz+kCd+0AJyUDxNcFTsoCsbtA3gIVFL0CIRGgKMm2vp/nwWPxF0hT4isABCERAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAADhCJ4LeVLVyf05d+sylZy7dufSjS7OTk5OPgrXcufN/KpfO16zyfqhzmLGvEjOX7mWA9zgcVx3SwrXL916AvtwHa+TShUtT3cwvfxUCGSLuvJy4dLPh/H2Qgfhg4NIXOrw7l9659EJ6CMfVmPx+5vOCrYSgc6dlpi79SbBC50FquuG8+Qt2JAPQ3QUp65NLdY/jIkh1+LUgi/vwXLk/qWDz/6J/+Ov56p+9uCqXrtz2n7li/GvBoppzJvNzs8m5S9eyG/eyfM9KjGT1/V147tIH979duvf5UoB98gEq8c35dt03p5t/qumqzFvBohT1zpyXqUvnLr3U1ZLqIFU+bZekfInlXHoIefkq/9ma99jnnVVypiSFQei8/UmjC6rO3LZOfIiPuuoXLszn0Tn5h1kWX7RbV/mGDFKJfKtEsPo55zNCkMLWwgcwDlBVwfYXmnYqRypcmC+jC/PULIvbjhrZ0q6ClMnflwyvo+PuLAUSpLA194EZRx+8unD7qaaN5UiFC9qe108dy3+QLe06SIV9xMfdWZoiSOX5lSApfMBqM+vGNYhOJFO4CCoz61vzutbCgPcU6LK/Um1m/zNezaXvzPTpIzlX/rj/aqZ9H8RasDWC1Hrn0XTpHZsL83oi7TuDF3KcavPaX9g3iXXemNf+Yv9SHoe4c+fngq0RpNazF8bElaJmkilRiroMvYgnZp4vIQzSD+gR8QHnGzP9MT6vbtr/ic9Vr86S+2SOe2ZmE6QGQJBKCNULG0BupMxKKcpUE22Vz+d/NA3ooap3Ksv/2c94s251l/5tpkePqHo8EwyKIJV2Fk1/J5lSpSjz+n3HfuK8LnTeH8unRrag897yi7wO0V/LFzVeRfM2ndebaNs/CoC56C7NtHDbqdn2tmP5uCOvM13Vq4+VtrtSNLJn2n4M5kPG+mOz/k99q8e6h7t7sjzmW7OfHzKOi7t7HShJpdlq2EwyabsUlXqkY+XOlWzgqom+5DUxsy56Xqg2GPqn9xvZI10+7V8tZslq1Te5maze+fP/95k8fPY9ZTSDARCk0mwg+I/ks21RPhikLkT7wR1lBJ3X0XEV3RVMBM5G9s9X1+LG75wqtA/S9yaPB1vlC4H4PJr9vQBDS1SNcp/DOo+2e5G5XpWRd9ybuZYM4X+Zmu0O8uygtqt67wq2exdVh4pLkrq/zpzTaB+nGcdFda8DJam2KprOLbLnlKJy9pfSRMeRW5pqZH0j/l5oe8QDP+N97uYu/T2ady4PTPgf/XtSmdkTBj4cBkGq26xrhVCyqcyspiC/30iH0MfKVvtq7RhoLXyL23UuS/p6DSiupt0XBHDPX+i2yvd7eSBCSciPyOpLqPaLw0etrwXYBW2PXFBnbDM2608L8z+X/GOz+7nbVPWJqh5FdyiHohtGPCjII354t7hKpLsbquVa04MgNgV5Ud3rwKB3bVXJytp+xq+R3bk0+1o0or9OHJPvj1SZWYccbK82r+M7djn8Nv+SZR8rX5o6l+3Os8/DjxHW59GkdYPeLVzu++4pjoy2+ybVHeuPS0osukVJKmwfN6KfRssfRGN5OJaNIx4U5nPXNx9l+OBHjTaptrihfG2bUfhA1WZWI+XuCtdvZPUYrxLLq/B6JgdoLPc0PeLBRPqz7ViVPozHZBbPGH7lSk+/LRklA/mo7nV7tmFZY17n3tGLqwr/lQK+Ed1doD7wLILTohHd9zuqZLWx/M2BGssXbOdLH7W+36KkYPurLUZGmEh/76WsD9zCLPz9yN07HIRm9pNKrJf1pL62+0nV0oO2B+SLHaSx3Bxf16/BbOungmPZeT+pPpTqXhaqe21xda9as15jXs8k/yHkKpru+23cVY17KQeiyxEPKtmdxzQyArZAkIqEPkk2ULXapMK3nS05fVtQrfo8sb8+ftex/JUcTmrEg13s4xvBk0ebVNpMlg+K1onlTbTujeSrzOtepagQJBtZPQaf7BhV/i5lfcDGXBtE/WgQf5Dt+cDk+1nVYdq3x434WXIcHW3f5h+ZZXFb1FVBvqNo217dA7TdqbMy+dtb9VPd8+ifa9p/sp5/zMz7dWnetEk9blT30uISji0VNNGydaNLptTR9EQKhYurNrN+edwllChsW1Ul6V9d3qXUiAe5z+rluIn29WAekwH2Zl2JJ1GKKioJaftHJKvC7eOOmtM1642j/ZzKnmi74+Ugv0Ic5T+OSh+jjm0oSeHp0cRzctsEGW0HuLEUyt2/zn/mXXcVKDYcn0/xD39mdc0o3EdRlY8ghSdJ24+vvI2nC/O7iLYvuni13b+q6Vj/uuRCHoKmx3+qZEDhwn4WnYsPGdsQpPD06ObOiFVBPnEpqnTc9FF0LP71KGMbW+3qPUZ45jGmRjzYSQlOC38tmCD1uNFwvtm6DpE3uf2iQmCIq3aNlIlHNWgybrv75X820/7xnr/JbtXmdc445n3Foynwa8E4XtpuB1ItG773Ntr2WgroFo31pSWObWiPBu0t9rWo8q2UFDvWpySFp0nn1aY40ExdeqHrG679NhfaHhDttvTC1XY1ryrYNlUF+6QDBw8dYHC7Hvv0QfEmusBrWX98BCk8XWsC1cJYzY9uhunUaI19AtRFlEfxnbLEheA1MiBN33H7UnZI02NEXWWuS5DC06Ttnui5/HalAcpX82ywu5UedFk1mu7qYtD2iAfZIxQMsN/OmwNKkHrUaDgv4Bqr/W3855LfIDxx6Qu/XY/ny3ypyW8zC+kr6cHt1//xA+t9bfL60aW/yAB0ObidmPx31WDe2r3Mf4B1FtK9rB95wZ5Ln0oHG9wVfxwzkxA5EfQSvrFrmT/Q618vRkvwAcA/VjPhwVcAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAwJH7H5NlZI/0GQ+cAAAAAElFTkSuQmCC', - ...customProps, - }; -}; - -describe('AggregatorLogo', () => { - it('renders the component with initial props', () => { - const { container } = renderWithProvider( - , - ); - expect(container).toMatchSnapshot(); - }); -}); diff --git a/ui/pages/swaps/loading-swaps-quotes/loading-swaps-quotes.js b/ui/pages/swaps/loading-swaps-quotes/loading-swaps-quotes.js index 1e8d389f6..32b139c5e 100644 --- a/ui/pages/swaps/loading-swaps-quotes/loading-swaps-quotes.js +++ b/ui/pages/swaps/loading-swaps-quotes/loading-swaps-quotes.js @@ -4,7 +4,6 @@ import { useDispatch, useSelector } from 'react-redux'; import PropTypes from 'prop-types'; import { shuffle } from 'lodash'; import { useHistory } from 'react-router-dom'; -import classnames from 'classnames'; import { navigateBackToBuildQuote, getFetchParams, @@ -19,44 +18,6 @@ import { MetaMetricsContext } from '../../../contexts/metametrics.new'; import Mascot from '../../../components/ui/mascot'; import SwapsFooter from '../swaps-footer'; import BackgroundAnimation from './background-animation'; -import AggregatorLogo from './aggregator-logo'; - -// These locations reference where we want the top-left corner of the logo div to appear in relation to the -// centre point of the fox -const AGGREGATOR_LOCATIONS = [ - { x: -125, y: -75 }, - { x: 30, y: -75 }, - { x: -145, y: 0 }, - { x: 50, y: 0 }, - { x: -135, y: 46 }, - { x: 40, y: 46 }, -]; - -function getRandomLocations(numberOfLocations) { - const randomLocations = shuffle(AGGREGATOR_LOCATIONS); - if (numberOfLocations <= AGGREGATOR_LOCATIONS.length) { - return randomLocations.slice(0, numberOfLocations); - } - const numberOfExtraLocations = - numberOfLocations - AGGREGATOR_LOCATIONS.length; - return [...randomLocations, ...getRandomLocations(numberOfExtraLocations)]; -} - -function getMascotTarget(aggregatorName, centerPoint, aggregatorLocationMap) { - const location = aggregatorLocationMap[aggregatorName]; - - if (!location || !centerPoint) { - return centerPoint ?? {}; - } - - // The aggregator logos are 94px x 40px. For the fox to look at the center of each logo, the target needs to be - // the coordinates for the centre point of the fox + the desired top and left coordinates of the logo + half - // the height and width of the logo. - return { - x: location.x + centerPoint.x + 47, - y: location.y + centerPoint.y + 20, - }; -} export default function LoadingSwapsQuotes({ aggregatorMetadata, @@ -97,20 +58,6 @@ export default function LoadingSwapsQuotes({ const currentMascotContainer = mascotContainer.current; const [quoteCount, updateQuoteCount] = useState(0); - // is an array of randomized items from AGGREGATOR_LOCATIONS, containing - // numberOfQuotes number of items it is randomized so that the order in - // which the fox looks at locations is random - const [aggregatorLocations] = useState(() => - getRandomLocations(numberOfQuotes), - ); - const _aggregatorLocationMap = aggregatorNames.reduce( - (nameLocationMap, name, index) => ({ - ...nameLocationMap, - [name]: aggregatorLocations[index], - }), - {}, - ); - const [aggregatorLocationMap] = useState(_aggregatorLocationMap); const [midPointTarget, setMidpointTarget] = useState(null); useEffect(() => { @@ -122,7 +69,7 @@ export default function LoadingSwapsQuotes({ if (loadingComplete) { // If loading is complete, but the quoteCount is not, we quickly display the remaining logos/names/fox looks. 0.2s each - timeoutLength = 200; + timeoutLength = 20; } else { // If loading is not complete, we display remaining logos/names/fox looks at random intervals between 0.5s and 2s, to simulate the // sort of loading a user would experience in most async scenarios @@ -167,13 +114,7 @@ export default function LoadingSwapsQuotes({
- - {quoteCount === numberOfQuotes - ? t('swapFinalizing') - : t('swapCheckingQuote', [ - aggregatorMetadata[aggregatorNames[quoteCount]].title, - ])} - + {t('swapFetchingQuotes')}
- {currentMascotContainer && - midPointTarget && - aggregatorNames.map((aggName) => ( -
- -
- ))}
{ store, ); expect(getByText('Quote 1 of 2')).toBeInTheDocument(); - expect(getByText('Checking agg', { exact: false })).toBeInTheDocument(); expect(getByText('Back')).toBeInTheDocument(); }); }); diff --git a/ui/pages/swaps/swaps.util.js b/ui/pages/swaps/swaps.util.js index f7d052aee..e4a8cb4e6 100644 --- a/ui/pages/swaps/swaps.util.js +++ b/ui/pages/swaps/swaps.util.js @@ -76,7 +76,7 @@ const getBaseUrlForNewSwapsApi = (type, chainId) => { return `${v2ApiBaseUrl}/networks/${chainIdDecimal}`; }; -const getBaseApi = function ( +export const getBaseApi = function ( type, chainId = MAINNET_CHAIN_ID, useNewSwapsApi = false, @@ -84,6 +84,7 @@ const getBaseApi = function ( const baseUrl = useNewSwapsApi ? getBaseUrlForNewSwapsApi(type, chainId) : METASWAP_CHAINID_API_HOST_MAP[chainId]; + const chainIdDecimal = chainId && parseInt(chainId, 16); if (!baseUrl) { throw new Error(`Swaps API calls are disabled for chainId: ${chainId}`); } @@ -100,8 +101,9 @@ const getBaseApi = function ( return `${baseUrl}/aggregatorMetadata`; case 'gasPrices': return `${baseUrl}/gasPrices`; - case 'refreshTime': - return `${baseUrl}/quoteRefreshRate`; + case 'network': + // Only use v2 for this endpoint. + return `${SWAPS_API_V2_BASE_URL}/networks/${chainIdDecimal}`; default: throw new Error('getBaseApi requires an api call type'); } @@ -441,23 +443,6 @@ export async function fetchSwapsFeatureFlags() { return response; } -export async function fetchSwapsQuoteRefreshTime(chainId, useNewSwapsApi) { - const response = await fetchWithCache( - getBaseApi('refreshTime', chainId, useNewSwapsApi), - { method: 'GET' }, - { cacheRefreshTime: 600000 }, - ); - - // We presently use milliseconds in the UI - if (typeof response?.seconds === 'number' && response.seconds > 0) { - return response.seconds * 1000; - } - - throw new Error( - `MetaMask - refreshTime provided invalid response: ${response}`, - ); -} - export async function fetchTokenPrice(address) { const query = `contract_addresses=${address}&vs_currencies=eth`; diff --git a/ui/pages/swaps/view-quote/view-quote.js b/ui/pages/swaps/view-quote/view-quote.js index cdfbf0195..6b6f2e37e 100644 --- a/ui/pages/swaps/view-quote/view-quote.js +++ b/ui/pages/swaps/view-quote/view-quote.js @@ -35,6 +35,7 @@ import { getBackgroundSwapRouteState, swapsQuoteSelected, getSwapsQuoteRefreshTime, + getReviewSwapClickedTimestamp, } from '../../../ducks/swaps/swaps'; import { conversionRateSelector, @@ -56,6 +57,7 @@ import { setCustomApproveTxData, setSwapsErrorKey, showModal, + setSwapsQuotesPollingLimitEnabled, } from '../../../store/actions'; import { ASSET_ROUTE, @@ -105,6 +107,8 @@ export default function ViewQuote() { const [warningHidden, setWarningHidden] = useState(false); const [originalApproveAmount, setOriginalApproveAmount] = useState(null); const [showEditGasPopover, setShowEditGasPopover] = useState(false); + // We need to have currentTimestamp in state, otherwise it would change with each rerender. + const [currentTimestamp] = useState(Date.now()); const [ acknowledgedPriceDifference, @@ -150,6 +154,7 @@ export default function ViewQuote() { const defaultSwapsToken = useSelector(getSwapsDefaultToken); const chainId = useSelector(getCurrentChainId); const nativeCurrencySymbol = useSelector(getNativeCurrency); + const reviewSwapClickedTimestamp = useSelector(getReviewSwapClickedTimestamp); let gasFeeInputs; if (networkAndAccountSupports1559) { @@ -371,6 +376,9 @@ export default function ViewQuote() { const showInsufficientWarning = (balanceError || tokenBalanceNeeded || ethBalanceNeeded) && !warningHidden; + const hardwareWalletUsed = useSelector(isHardwareWallet); + const hardwareWalletType = useSelector(getHardwareWalletType); + const numberOfQuotes = Object.values(quotes).length; const bestQuoteReviewedEventSent = useRef(); const eventObjectBase = { @@ -384,10 +392,10 @@ export default function ViewQuote() { response_time: fetchParams?.responseTime, best_quote_source: topQuote?.aggregator, available_quotes: numberOfQuotes, + is_hardware_wallet: hardwareWalletUsed, + hardware_wallet_type: hardwareWalletType, }; - const hardwareWalletUsed = useSelector(isHardwareWallet); - const hardwareWalletType = useSelector(getHardwareWalletType); const allAvailableQuotesOpened = useNewMetricEvent({ event: 'All Available Quotes Opened', category: 'swaps', @@ -398,8 +406,6 @@ export default function ViewQuote() { usedQuote?.aggregator === topQuote?.aggregator ? null : usedQuote?.aggregator, - is_hardware_wallet: hardwareWalletUsed, - hardware_wallet_type: hardwareWalletType, }, }); const quoteDetailsOpened = useNewMetricEvent({ @@ -412,8 +418,6 @@ export default function ViewQuote() { usedQuote?.aggregator === topQuote?.aggregator ? null : usedQuote?.aggregator, - is_hardware_wallet: hardwareWalletUsed, - hardware_wallet_type: hardwareWalletType, }, }); const editSpendLimitOpened = useNewMetricEvent({ @@ -424,8 +428,6 @@ export default function ViewQuote() { custom_spend_limit_set: originalApproveAmount === approveAmount, custom_spend_limit_amount: originalApproveAmount === approveAmount ? null : approveAmount, - is_hardware_wallet: hardwareWalletUsed, - hardware_wallet_type: hardwareWalletType, }, }); @@ -435,10 +437,18 @@ export default function ViewQuote() { sensitiveProperties: { ...eventObjectBase, network_fees: feeInFiat, - is_hardware_wallet: hardwareWalletUsed, - hardware_wallet_type: hardwareWalletType, }, }); + + const viewQuotePageLoadedEvent = useNewMetricEvent({ + event: 'View Quote Page Loaded', + category: 'swaps', + sensitiveProperties: { + ...eventObjectBase, + response_time: currentTimestamp - reviewSwapClickedTimestamp, + }, + }); + useEffect(() => { if ( !bestQuoteReviewedEventSent.current && @@ -653,6 +663,14 @@ export default function ViewQuote() { setShowEditGasPopover(false); }; + useEffect(() => { + // Thanks to the next line we will only do quotes polling 3 times before showing a Quote Timeout modal. + dispatch(setSwapsQuotesPollingLimitEnabled(true)); + if (reviewSwapClickedTimestamp) { + viewQuotePageLoadedEvent(); + } + }, [dispatch, viewQuotePageLoadedEvent, reviewSwapClickedTimestamp]); + return (
{ diff --git a/ui/store/actions.js b/ui/store/actions.js index 09e7362c7..a1705cc54 100644 --- a/ui/store/actions.js +++ b/ui/store/actions.js @@ -2161,6 +2161,13 @@ export function setSwapsTokens(tokens) { }; } +export function clearSwapsQuotes() { + return async (dispatch) => { + await promisifiedBackground.clearSwapsQuotes(); + await forceUpdateMetamaskState(dispatch); + }; +} + export function resetBackgroundSwapsState() { return async (dispatch) => { const id = await promisifiedBackground.resetSwapsState(); @@ -2214,6 +2221,15 @@ export function updateSwapsUserFeeLevel(swapsCustomUserFeeLevel) { }; } +export function setSwapsQuotesPollingLimitEnabled(quotesPollingLimitEnabled) { + return async (dispatch) => { + await promisifiedBackground.setSwapsQuotesPollingLimitEnabled( + quotesPollingLimitEnabled, + ); + await forceUpdateMetamaskState(dispatch); + }; +} + export function customSwapsGasParamsUpdated(gasLimit, gasPrice) { return async (dispatch) => { await promisifiedBackground.setSwapsTxGasPrice(gasPrice);