From 066c568aca5d3a41ff17547193f02ea41aabfe94 Mon Sep 17 00:00:00 2001 From: Daniel <80175477+dan437@users.noreply.github.com> Date: Thu, 3 Aug 2023 22:49:50 +0200 Subject: [PATCH] Improvements to Swaps quote auto-selection logic, fix and edge case with zero-balance tokens (#20388) * Add Token To into assets again (reverting commit 51f46eb65f48bdf4980f400a589bf1ac63a65222 ) * Update cleanup for an unswapped Token To from the Tokens list * Call "setLatestAddedTokenTo" conditionally * Update an E2E test for insufficient balance notification --- test/e2e/swaps/swaps-notifications.spec.js | 2 +- ui/ducks/swaps/swaps.js | 71 ++++++++++--------- ui/pages/swaps/build-quote/build-quote.js | 14 +++- .../swaps/build-quote/build-quote.test.js | 1 + ui/pages/swaps/index.js | 32 +++++++++ .../prepare-swap-page/prepare-swap-page.js | 14 +++- .../prepare-swap-page.test.js | 1 + 7 files changed, 99 insertions(+), 36 deletions(-) diff --git a/test/e2e/swaps/swaps-notifications.spec.js b/test/e2e/swaps/swaps-notifications.spec.js index 25c0595b8..04d52c135 100644 --- a/test/e2e/swaps/swaps-notifications.spec.js +++ b/test/e2e/swaps/swaps-notifications.spec.js @@ -111,7 +111,7 @@ describe('Swaps - notifications', function () { }); await checkNotification(driver, { title: 'Insufficient balance', - text: 'You need 50 more TESTETH to complete this swap', + text: 'You need 43.4467 more TESTETH to complete this swap', }); await reviewQuote(driver, { swapFrom: 'TESTETH', diff --git a/ui/ducks/swaps/swaps.js b/ui/ducks/swaps/swaps.js index a767fd8e5..2c50c8025 100644 --- a/ui/ducks/swaps/swaps.js +++ b/ui/ducks/swaps/swaps.js @@ -124,6 +124,7 @@ const initialState = { currentSmartTransactionsError: '', swapsSTXLoading: false, transactionSettingsOpened: false, + latestAddedTokenTo: '', }; const slice = createSlice({ @@ -153,6 +154,9 @@ const slice = createSlice({ setFetchingQuotes: (state, action) => { state.fetchingQuotes = action.payload; }, + setLatestAddedTokenTo: (state, action) => { + state.latestAddedTokenTo = action.payload; + }, setFromToken: (state, action) => { state.fromToken = action.payload; }, @@ -248,6 +252,8 @@ export const getToToken = (state) => state.swaps.toToken; export const getFetchingQuotes = (state) => state.swaps.fetchingQuotes; +export const getLatestAddedTokenTo = (state) => state.swaps.latestAddedTokenTo; + export const getQuotesFetchStartTime = (state) => state.swaps.quotesFetchStartTime; @@ -482,6 +488,7 @@ const { setAggregatorMetadata, setBalanceError, setFetchingQuotes, + setLatestAddedTokenTo, setFromToken, setFromTokenError, setFromTokenInputValue, @@ -505,6 +512,7 @@ export { setAggregatorMetadata, setBalanceError, setFetchingQuotes, + setLatestAddedTokenTo, setFromToken as setSwapsFromToken, setFromTokenError, setFromTokenInputValue, @@ -668,7 +676,12 @@ export const fetchQuotesAndSetQuoteState = ( iconUrl: fromTokenIconUrl, balance: fromTokenBalance, } = selectedFromToken; - const { address: toTokenAddress, symbol: toTokenSymbol } = selectedToToken; + const { + address: toTokenAddress, + symbol: toTokenSymbol, + decimals: toTokenDecimals, + iconUrl: toTokenIconUrl, + } = selectedToToken; // 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) { @@ -679,6 +692,30 @@ export const fetchQuotesAndSetQuoteState = ( const contractExchangeRates = getTokenExchangeRates(state); + if ( + toTokenAddress && + toTokenSymbol !== swapsDefaultToken.symbol && + contractExchangeRates[toTokenAddress] === undefined && + !isTokenAlreadyAdded(toTokenAddress, getTokens(state)) + ) { + await dispatch( + addToken( + toTokenAddress, + toTokenSymbol, + toTokenDecimals, + toTokenIconUrl, + true, + ), + ); + await dispatch(setLatestAddedTokenTo(toTokenAddress)); + } else { + const latestAddedTokenTo = getLatestAddedTokenTo(state); + // Only reset the latest added Token To if it's a different token. + if (latestAddedTokenTo !== toTokenAddress) { + await dispatch(setLatestAddedTokenTo('')); + } + } + if ( fromTokenAddress && fromTokenSymbol !== swapsDefaultToken.symbol && @@ -843,36 +880,6 @@ export const fetchQuotesAndSetQuoteState = ( }; }; -const addTokenTo = (dispatch, state) => { - const fetchParams = getFetchParams(state); - const swapsDefaultToken = getSwapsDefaultToken(state); - const contractExchangeRates = getTokenExchangeRates(state); - const selectedToToken = - getToToken(state) || fetchParams?.metaData?.destinationTokenInfo || {}; - const { - address: toTokenAddress, - symbol: toTokenSymbol, - decimals: toTokenDecimals, - iconUrl: toTokenIconUrl, - } = selectedToToken; - if ( - toTokenAddress && - toTokenSymbol !== swapsDefaultToken.symbol && - contractExchangeRates[toTokenAddress] === undefined && - !isTokenAlreadyAdded(toTokenAddress, getTokens(state)) - ) { - dispatch( - addToken( - toTokenAddress, - toTokenSymbol, - toTokenDecimals, - toTokenIconUrl, - true, - ), - ); - } -}; - export const signAndSendSwapsSmartTransaction = ({ unsignedTransaction, trackEvent, @@ -972,7 +979,6 @@ export const signAndSendSwapsSmartTransaction = ({ dispatch(setCurrentSmartTransactionsError(StxErrorTypes.unavailable)); return; } - addTokenTo(dispatch, state); if (approveTxParams) { updatedApproveTxParams.gas = `0x${decimalToHex( fees.approvalTxFees?.gasLimit || 0, @@ -1216,7 +1222,6 @@ export const signAndSendTransactions = ( history.push(AWAITING_SIGNATURES_ROUTE); } - addTokenTo(dispatch, state); if (approveTxParams) { if (networkAndAccountSupports1559) { approveTxParams.maxFeePerGas = maxFeePerGas; diff --git a/ui/pages/swaps/build-quote/build-quote.js b/ui/pages/swaps/build-quote/build-quote.js index b0b9d3bbe..3a03307cc 100644 --- a/ui/pages/swaps/build-quote/build-quote.js +++ b/ui/pages/swaps/build-quote/build-quote.js @@ -48,6 +48,7 @@ import { getIsFeatureFlagLoaded, getCurrentSmartTransactionsError, getSmartTransactionFees, + getLatestAddedTokenTo, } from '../../../ducks/swaps/swaps'; import { getSwapsDefaultToken, @@ -84,6 +85,7 @@ import { import { resetSwapsPostFetchState, + ignoreTokens, setBackgroundSwapRouteState, clearSwapsQuotes, stopPollingForQuotes, @@ -144,6 +146,7 @@ export default function BuildQuote({ const tokenList = useSelector(getTokenList, isEqual); const quotes = useSelector(getQuotes, isEqual); const areQuotesPresent = Object.keys(quotes).length > 0; + const latestAddedTokenTo = useSelector(getLatestAddedTokenTo, isEqual); const tokenConversionRates = useSelector(getTokenExchangeRates, isEqual); const conversionRate = useSelector(getConversionRate); @@ -347,12 +350,21 @@ export default function BuildQuote({ ? getURLHostName(blockExplorerTokenLink) : t('etherscan'); + const { address: toAddress } = toToken || {}; const onToSelect = useCallback( (token) => { + if (latestAddedTokenTo && token.address !== toAddress) { + dispatch( + ignoreTokens({ + tokensToIgnore: toAddress, + dontShowLoadingIndicator: true, + }), + ); + } dispatch(setSwapToToken(token)); setVerificationClicked(false); }, - [dispatch], + [dispatch, latestAddedTokenTo, toAddress], ); const hideDropdownItemIf = useCallback( diff --git a/ui/pages/swaps/build-quote/build-quote.test.js b/ui/pages/swaps/build-quote/build-quote.test.js index e09ec1149..ea8c2cc70 100644 --- a/ui/pages/swaps/build-quote/build-quote.test.js +++ b/ui/pages/swaps/build-quote/build-quote.test.js @@ -29,6 +29,7 @@ const createProps = (customProps = {}) => { setBackgroundConnection({ resetPostFetchState: jest.fn(), + ignoreTokens: jest.fn(), setBackgroundSwapRouteState: jest.fn(), clearSwapsQuotes: jest.fn(), stopPollingForQuotes: jest.fn(), diff --git a/ui/pages/swaps/index.js b/ui/pages/swaps/index.js index 9aa9ad2a7..8a127063e 100644 --- a/ui/pages/swaps/index.js +++ b/ui/pages/swaps/index.js @@ -50,6 +50,7 @@ import { navigateBackToBuildQuote, getSwapRedesignEnabled, setTransactionSettingsOpened, + getLatestAddedTokenTo, } from '../../ducks/swaps/swaps'; import { checkNetworkAndAccountSupports1559, @@ -79,6 +80,7 @@ import { import { resetBackgroundSwapsState, setSwapsTokens, + ignoreTokens, setBackgroundSwapRouteState, setSwapsErrorKey, } from '../../store/actions'; @@ -134,6 +136,7 @@ export default function Swap() { const routeState = useSelector(getBackgroundSwapRouteState); const selectedAccount = useSelector(getSelectedAccount, shallowEqual); const quotes = useSelector(getQuotes, isEqual); + const latestAddedTokenTo = useSelector(getLatestAddedTokenTo, isEqual); const txList = useSelector(currentNetworkTxListSelector, shallowEqual); const tradeTxId = useSelector(getTradeTxId); const approveTxId = useSelector(getApproveTxId); @@ -209,6 +212,32 @@ export default function Swap() { swapsErrorKey = SWAP_FAILED_ERROR; } + const clearTemporaryTokenRef = useRef(); + useEffect(() => { + clearTemporaryTokenRef.current = () => { + if (latestAddedTokenTo && (!isAwaitingSwapRoute || conversionError)) { + dispatch( + ignoreTokens({ + tokensToIgnore: latestAddedTokenTo, + dontShowLoadingIndicator: true, + }), + ); + } + }; + }, [ + conversionError, + dispatch, + latestAddedTokenTo, + destinationTokenInfo, + fetchParams, + isAwaitingSwapRoute, + ]); + useEffect(() => { + return () => { + clearTemporaryTokenRef.current(); + }; + }, []); + // eslint-disable-next-line useEffect(() => { if (!isSwapsChain) { @@ -283,6 +312,7 @@ export default function Swap() { const beforeUnloadEventAddedRef = useRef(); useEffect(() => { const fn = () => { + clearTemporaryTokenRef.current(); if (isLoadingQuotesRoute) { dispatch(prepareToLeaveSwaps()); } @@ -349,6 +379,7 @@ export default function Swap() { } const redirectToDefaultRoute = async () => { + clearTemporaryTokenRef.current(); history.push({ pathname: DEFAULT_ROUTE, state: { stayOnHomePage: true }, @@ -403,6 +434,7 @@ export default function Swap() {
{ + clearTemporaryTokenRef.current(); dispatch(clearSwapsState()); await dispatch(resetBackgroundSwapsState()); history.push(DEFAULT_ROUTE); diff --git a/ui/pages/swaps/prepare-swap-page/prepare-swap-page.js b/ui/pages/swaps/prepare-swap-page/prepare-swap-page.js index 16a7da28a..c5cccaa1a 100644 --- a/ui/pages/swaps/prepare-swap-page/prepare-swap-page.js +++ b/ui/pages/swaps/prepare-swap-page/prepare-swap-page.js @@ -54,6 +54,7 @@ import { getAggregatorMetadata, getTransactionSettingsOpened, setTransactionSettingsOpened, + getLatestAddedTokenTo, } from '../../../ducks/swaps/swaps'; import { getSwapsDefaultToken, @@ -92,6 +93,7 @@ import { } from '../../../../shared/constants/swaps'; import { resetSwapsPostFetchState, + ignoreTokens, clearSwapsQuotes, stopPollingForQuotes, setSmartTransactionsOptInStatus, @@ -183,6 +185,7 @@ export default function PrepareSwapPage({ const rpcPrefs = useSelector(getRpcPrefsForCurrentProvider, shallowEqual); const tokenList = useSelector(getTokenList, isEqual); const quotes = useSelector(getQuotes, isEqual); + const latestAddedTokenTo = useSelector(getLatestAddedTokenTo, isEqual); const numberOfQuotes = Object.keys(quotes).length; const areQuotesPresent = numberOfQuotes > 0; const swapsErrorKey = useSelector(getSwapsErrorKey); @@ -450,12 +453,21 @@ export default function PrepareSwapPage({ ? getURLHostName(blockExplorerTokenLink) : t('etherscan'); + const { address: toAddress } = toToken || {}; const onToSelect = useCallback( (token) => { + if (latestAddedTokenTo && token.address !== toAddress) { + dispatch( + ignoreTokens({ + tokensToIgnore: toAddress, + dontShowLoadingIndicator: true, + }), + ); + } dispatch(setSwapToToken(token)); setVerificationClicked(false); }, - [dispatch], + [dispatch, latestAddedTokenTo, toAddress], ); const tokensWithBalancesFromToken = tokensWithBalances.find((token) => diff --git a/ui/pages/swaps/prepare-swap-page/prepare-swap-page.test.js b/ui/pages/swaps/prepare-swap-page/prepare-swap-page.test.js index 61255621f..9f0a499f7 100644 --- a/ui/pages/swaps/prepare-swap-page/prepare-swap-page.test.js +++ b/ui/pages/swaps/prepare-swap-page/prepare-swap-page.test.js @@ -27,6 +27,7 @@ const createProps = (customProps = {}) => { setBackgroundConnection({ resetPostFetchState: jest.fn(), + ignoreTokens: jest.fn(), setBackgroundSwapRouteState: jest.fn(), clearSwapsQuotes: jest.fn(), stopPollingForQuotes: jest.fn(),