From 549140f6f53d7d57c64518f7bee2094f41f45887 Mon Sep 17 00:00:00 2001 From: Dan J Miller Date: Fri, 9 Oct 2020 12:26:23 -0230 Subject: [PATCH] Remove max mode for eth feature from swaps (#9531) * Remove max mode for eth feature from swaps * Fix unit tests after removing maxMode from swaps --- app/scripts/controllers/swaps.js | 49 ------------------- app/scripts/metamask-controller.js | 1 - test/unit/app/controllers/swaps-test.js | 10 ---- ui/app/ducks/swaps/swaps.js | 49 ++----------------- ui/app/pages/swaps/build-quote/build-quote.js | 20 ++++---- ui/app/pages/swaps/index.js | 3 +- ui/app/pages/swaps/view-quote/view-quote.js | 14 +----- ui/app/store/actions.js | 7 --- 8 files changed, 14 insertions(+), 139 deletions(-) diff --git a/app/scripts/controllers/swaps.js b/app/scripts/controllers/swaps.js index fc9f5823d..95764d5f9 100644 --- a/app/scripts/controllers/swaps.js +++ b/app/scripts/controllers/swaps.js @@ -3,7 +3,6 @@ import log from 'loglevel' import BigNumber from 'bignumber.js' import ObservableStore from 'obs-store' import { mapValues } from 'lodash' -import ethUtil from 'ethereumjs-util' import abi from 'human-standard-token-abi' import { calcTokenAmount } from '../../../ui/app/helpers/utils/token-util' import { calcGasTotal } from '../../../ui/app/pages/send/send.utils' @@ -55,7 +54,6 @@ const initialState = { tokens: null, tradeTxId: null, approveTxId: null, - maxMode: false, quotesLastFetched: null, customMaxGas: '', customGasPrice: null, @@ -179,10 +177,6 @@ export default class SwapsController { if (!approvalRequired && !fetchParams?.balanceError) { newQuotes = await this.getAllQuotesWithGasEstimates(newQuotes) - if (fetchParamsMetaData.maxMode && fetchParams.sourceToken === ETH_SWAPS_TOKEN_ADDRESS) { - newQuotes = await this._modifyAndFilterValuesForMaxEthMode(newQuotes, fetchParamsMetaData.accountBalance) - } - if (Object.values(newQuotes).length === 0) { this.setSwapsErrorKey(QUOTES_NOT_AVAILABLE_ERROR) } else { @@ -345,11 +339,6 @@ export default class SwapsController { this.store.updateState({ swapsState: { ...swapsState, tradeTxId } }) } - setMaxMode (maxMode) { - const { swapsState } = this.store.getState() - this.store.updateState({ swapsState: { ...swapsState, maxMode } }) - } - setQuotesLastFetched (quotesLastFetched) { const { swapsState } = this.store.getState() this.store.updateState({ swapsState: { ...swapsState, quotesLastFetched } }) @@ -584,42 +573,4 @@ export default class SwapsController { } } - async _modifyAndFilterValuesForMaxEthMode (newQuotes, accountBalance) { - const { - swapsState: { customGasPrice }, - } = this.store.getState() - - const usedGasPrice = customGasPrice || await this._getEthersGasPrice() - - const mappedNewQuotes = {} - Object.values(newQuotes).forEach((quote) => { - const oldSourceAmount = quote.sourceAmount - - const gasTotalInWeiHex = calcGasTotal((new BigNumber(quote.maxGas, 10)).toString(16), usedGasPrice) - const newSourceAmount = (new BigNumber(accountBalance, 16)).minus(gasTotalInWeiHex, 16).toString(10) - - const newOldRatio = (new BigNumber(newSourceAmount, 10)).div(oldSourceAmount, 10) - const oldNewDifference = (new BigNumber(oldSourceAmount, 10)).minus(newSourceAmount, 10).toString(10) - - const oldDestinationAmount = quote.destinationAmount - const newDestinationAmount = (new BigNumber(oldDestinationAmount, 10)).times(newOldRatio).toString(10) - - const oldValue = quote.trade.value - const newValue = (new BigNumber(oldValue, 16)).minus(oldNewDifference, 10).toString(16) - - if ((new BigNumber(newSourceAmount, 10).gt(0))) { - mappedNewQuotes[quote.aggregator] = { - ...quote, - trade: { - ...quote.trade, - value: ethUtil.addHexPrefix(newValue), - }, - destinationAmount: newDestinationAmount, - sourceAmount: newSourceAmount, - } - } - }) - - return mappedNewQuotes - } } diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 22b326c7d..130500dab 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -605,7 +605,6 @@ export default class MetamaskController extends EventEmitter { setSwapsTokens: nodeify(swapsController.setSwapsTokens, swapsController), setApproveTxId: nodeify(swapsController.setApproveTxId, swapsController), setTradeTxId: nodeify(swapsController.setTradeTxId, swapsController), - setMaxMode: nodeify(swapsController.setMaxMode, swapsController), setSwapsTxGasPrice: nodeify(swapsController.setSwapsTxGasPrice, swapsController), setSwapsTxGasLimit: nodeify(swapsController.setSwapsTxGasLimit, swapsController), safeRefetchQuotes: nodeify(swapsController.safeRefetchQuotes, swapsController), diff --git a/test/unit/app/controllers/swaps-test.js b/test/unit/app/controllers/swaps-test.js index 171652302..703283f82 100644 --- a/test/unit/app/controllers/swaps-test.js +++ b/test/unit/app/controllers/swaps-test.js @@ -101,7 +101,6 @@ const EMPTY_INIT_STATE = { tokens: null, tradeTxId: null, approveTxId: null, - maxMode: false, quotesLastFetched: null, customMaxGas: '', customGasPrice: null, @@ -197,15 +196,6 @@ describe('SwapsController', function () { ) }) - it('should set max mode', function () { - const maxMode = true - swapsController.setMaxMode(maxMode) - assert.strictEqual( - swapsController.store.getState().swapsState.maxMode, - maxMode, - ) - }) - it('should set swaps tx gas price', function () { const gasPrice = 1 swapsController.setSwapsTxGasPrice(gasPrice) diff --git a/ui/app/ducks/swaps/swaps.js b/ui/app/ducks/swaps/swaps.js index 5384e6150..171256720 100644 --- a/ui/app/ducks/swaps/swaps.js +++ b/ui/app/ducks/swaps/swaps.js @@ -21,10 +21,9 @@ import { setSwapsLiveness, } from '../../store/actions' import { AWAITING_SWAP_ROUTE, BUILD_QUOTE_ROUTE, LOADING_QUOTES_ROUTE, SWAPS_ERROR_ROUTE, SWAPS_MAINTENANCE_ROUTE } from '../../helpers/constants/routes' -import { fetchTradesInfo, fetchSwapsFeatureLiveness } from '../../pages/swaps/swaps.util' +import { fetchSwapsFeatureLiveness } from '../../pages/swaps/swaps.util' import { calcGasTotal } from '../../pages/send/send.utils' -import { decimalToHex, getValueFromWeiHex, hexMax, decGWEIToHexWEI, hexWEIToDecETH } from '../../helpers/utils/conversions.util' -import { constructTxParams } from '../../helpers/utils/util' +import { decimalToHex, getValueFromWeiHex, hexMax, decGWEIToHexWEI } from '../../helpers/utils/conversions.util' import { calcTokenAmount } from '../../helpers/utils/token-util' import { getFastPriceEstimateInHexWEI, @@ -132,8 +131,6 @@ export const getCustomSwapsGasPrice = (state) => state.metamask.swapsState.custo export const getFetchParams = (state) => state.metamask.swapsState.fetchParams -export const getMaxMode = (state) => state.metamask.swapsState.maxMode - export const getQuotes = (state) => state.metamask.swapsState.quotes export const getQuotesLastFetched = (state) => state.metamask.swapsState.quotesLastFetched @@ -317,8 +314,6 @@ export const fetchQuotesAndSetQuoteState = (history, inputValue, maxSlippage, me dispatch(setFromToken(selectedFromToken)) - const maxMode = getMaxMode(state) - metaMetricsEvent({ event: 'Quotes Requested', category: 'swaps', @@ -354,7 +349,6 @@ export const fetchQuotesAndSetQuoteState = (history, inputValue, maxSlippage, me sourceDecimals: fromTokenDecimals, }, { - maxMode, sourceTokenInfo, destinationTokenInfo, accountBalance: selectedAccount.balance, @@ -445,7 +439,7 @@ export const signAndSendTransactions = (history, metaMetricsEvent) => { history.push(AWAITING_SWAP_ROUTE) const usedQuote = getUsedQuote(state) - let usedTradeTxParams = usedQuote.trade + const usedTradeTxParams = usedQuote.trade const estimatedGasLimit = new BigNumber(usedQuote?.gasEstimate || decimalToHex(usedQuote?.averageGas || 0), 16) const estimatedGasLimitWithMultiplier = estimatedGasLimit.times(1.4, 10).round(0).toString(16) @@ -458,43 +452,6 @@ export const signAndSendTransactions = (history, metaMetricsEvent) => { const usedGasPrice = customConvertGasPrice || tradeTxParams?.gasPrice || fastGasEstimate usedTradeTxParams.gasPrice = usedGasPrice - const totalGasLimitForCalculation = (new BigNumber(usedTradeTxParams.gas, 16)).plus(usedQuote.approvalNeeded?.gas || '0x0', 16).toString(16) - const gasTotalInWeiHex = calcGasTotal(totalGasLimitForCalculation, usedGasPrice) - - const maxMode = getMaxMode(state) - const selectedAccount = getSelectedAccount(state) - if (maxMode && sourceTokenInfo.symbol === 'ETH') { - const ethBalance = selectedAccount.balance - - if ((new BigNumber(ethBalance, 16)).lte(gasTotalInWeiHex, 16)) { - // If somehow signAndSendTransactions was called when the users eth balance is less than the gas cost of a swap, an error has occured. - // The swap transaction should not be created. - dispatch(setSwapsErrorKey(SWAP_FAILED_ERROR)) - history.push(SWAPS_ERROR_ROUTE) - return - } - - const revisedTradeValueInHexWei = (new BigNumber(ethBalance, 16)).minus(gasTotalInWeiHex, 16).toString(16) - const revisedTradeValueInEth = hexWEIToDecETH(revisedTradeValueInHexWei) - const revisedQuotes = await fetchTradesInfo({ - sourceToken: sourceTokenInfo.address, - destinationToken: destinationTokenInfo.address, - slippage, - value: revisedTradeValueInEth, - exchangeList: usedQuote.aggregator, - fromAddress: selectedAccount.address, - timeout: 10000, - sourceDecimals: 18, - }) - const revisedQuote = Object.values(revisedQuotes)[0] - usedTradeTxParams = constructTxParams({ - ...revisedQuote.trade, - gas: decimalToHex(usedTradeTxParams.gas), - amount: decimalToHex(revisedQuote.trade.value), - gasPrice: tradeTxParams.gasPrice, - }) - } - const conversionRate = getConversionRate(state) const destinationValue = calcTokenAmount(usedQuote.destinationAmount, destinationTokenInfo.decimals || 18).toPrecision(8) const usedGasLimitEstimate = usedQuote?.gasEstimateWithRefund || (`0x${decimalToHex(usedQuote?.averageGas || 0)}`) diff --git a/ui/app/pages/swaps/build-quote/build-quote.js b/ui/app/pages/swaps/build-quote/build-quote.js index 71520a88a..b5869bc18 100644 --- a/ui/app/pages/swaps/build-quote/build-quote.js +++ b/ui/app/pages/swaps/build-quote/build-quote.js @@ -32,7 +32,7 @@ import { useEthFiatAmount } from '../../../hooks/useEthFiatAmount' import { ETH_SWAPS_TOKEN_OBJECT } from '../../../helpers/constants/swaps' -import { setMaxMode, resetSwapsPostFetchState, removeToken } from '../../../store/actions' +import { resetSwapsPostFetchState, removeToken } from '../../../store/actions' import { fetchTokenPrice, fetchTokenBalance } from '../swaps.util' import SwapsFooter from '../swaps-footer' @@ -139,7 +139,6 @@ export default function BuildQuote ({ }) } dispatch(setSwapsFromToken(token)) - dispatch(setMaxMode(false)) onInputChange(token?.address ? inputValue : '', token.string, token.decimals) } @@ -154,6 +153,7 @@ export default function BuildQuote ({ const hideDropdownItemIf = useCallback((item) => item.address === fromTokenAddress, [fromTokenAddress]) + // If the eth balance changes while on build quote, we update the selected from token useEffect(() => { if (fromToken?.address === ETH_SWAPS_TOKEN_OBJECT.address && (fromToken?.balance !== ethBalance)) { dispatch(setSwapsFromToken({ ...fromToken, balance: ethBalance, string: getValueFromWeiHex({ value: ethBalance, numberOfDecimals: 4, toDenomination: 'ETH' }) })) @@ -175,20 +175,18 @@ export default function BuildQuote ({
{t('swapSwapFrom')}
-
{ - dispatch(setMaxMode(true)) - onInputChange(fromTokenBalance || '0', fromTokenBalance) - }} - >{t('max')} -
+ {fromTokenSymbol !== 'ETH' && ( +
onInputChange(fromTokenBalance || '0', fromTokenBalance)} + >{t('max')} +
+ )}
{ - dispatch(setMaxMode(false)) onInputChange(value, fromTokenBalance) }} inputValue={inputValue} diff --git a/ui/app/pages/swaps/index.js b/ui/app/pages/swaps/index.js index fb1e8c517..45907859d 100644 --- a/ui/app/pages/swaps/index.js +++ b/ui/app/pages/swaps/index.js @@ -42,7 +42,7 @@ import { OFFLINE_FOR_MAINTENANCE, } from '../../helpers/constants/swaps' -import { resetBackgroundSwapsState, setSwapsTokens, setMaxMode, removeToken, setBackgroundSwapRouteState, setSwapsErrorKey } from '../../store/actions' +import { resetBackgroundSwapsState, setSwapsTokens, removeToken, setBackgroundSwapRouteState, setSwapsErrorKey } from '../../store/actions' import { getFastPriceEstimateInHexWEI, currentNetworkTxListSelector, getRpcPrefsForCurrentProvider } from '../../selectors' import { useNewMetricEvent } from '../../hooks/useMetricEvent' import { getValueFromWeiHex } from '../../helpers/utils/conversions.util' @@ -268,7 +268,6 @@ export default function Swap () { onInputChange={onInputChange} ethBalance={ethBalance} setMaxSlippage={setMaxSlippage} - setMaxMode={setMaxMode} selectedAccountAddress={selectedAccountAddress} maxSlippage={maxSlippage} /> diff --git a/ui/app/pages/swaps/view-quote/view-quote.js b/ui/app/pages/swaps/view-quote/view-quote.js index 03b2b8d63..00eac2cf9 100644 --- a/ui/app/pages/swaps/view-quote/view-quote.js +++ b/ui/app/pages/swaps/view-quote/view-quote.js @@ -19,7 +19,6 @@ import { setBalanceError, getQuotesLastFetched, getBalanceError, - getMaxMode, getCustomSwapsGas, getDestinationTokenInfo, getSwapsTradeTxParams, @@ -110,7 +109,6 @@ export default function ViewQuote () { const currentCurrency = useSelector(getCurrentCurrency) const swapsTokens = useSelector(getTokens) const balanceError = useSelector(getBalanceError) - const maxMode = useSelector(getMaxMode) const fetchParams = useSelector(getFetchParams) const approveTxParams = useSelector(getApproveTxParams) const selectedQuote = useSelector(getSelectedQuote) @@ -233,9 +231,6 @@ export default function ViewQuote () { (tokenCost).gt(new BigNumber(selectedFromToken.balance || '0x0')) ) - const insufficientEthForGas = (new BigNumber(gasTotalInWeiHex, 16)) - .gt(new BigNumber(ethBalance || '0x0')) - const insufficientEth = (ethCost).gt(new BigNumber(ethBalance || '0x0')) const tokenBalanceNeeded = insufficientTokens @@ -282,16 +277,9 @@ export default function ViewQuote () { } }, [originalApproveAmount, approveAmount]) - const allowedMaxModeSubmit = ( - maxMode && - sourceTokenSymbol === 'ETH' && - !insufficientEthForGas - ) - const showWarning = ( (balanceError || tokenBalanceNeeded || ethBalanceNeeded) && - !warningHidden && - !allowedMaxModeSubmit + !warningHidden ) const numberOfQuotes = Object.values(quotes).length diff --git a/ui/app/store/actions.js b/ui/app/store/actions.js index 9fba80d02..2c948366f 100644 --- a/ui/app/store/actions.js +++ b/ui/app/store/actions.js @@ -2180,13 +2180,6 @@ export function resetBackgroundSwapsState () { } } -export function setMaxMode (maxMode) { - return async (dispatch) => { - await promisifiedBackground.setMaxMode(maxMode) - await forceUpdateMetamaskState(dispatch) - } -} - export function setCustomApproveTxData (data) { return async (dispatch) => { await promisifiedBackground.setCustomApproveTxData(data)