diff --git a/app/scripts/controllers/swaps.js b/app/scripts/controllers/swaps.js index 3b964d9a8..c7691dffe 100644 --- a/app/scripts/controllers/swaps.js +++ b/app/scripts/controllers/swaps.js @@ -2,7 +2,7 @@ import { ethers } from 'ethers' import log from 'loglevel' import BigNumber from 'bignumber.js' import ObservableStore from 'obs-store' -import { mapValues } from 'lodash' +import { mapValues, cloneDeep } from 'lodash' 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' @@ -200,15 +200,12 @@ export default class SwapsController { if (Object.values(newQuotes).length === 0) { this.setSwapsErrorKey(QUOTES_NOT_AVAILABLE_ERROR) } else { - const topQuoteData = await this._findTopQuoteAndCalculateSavings( - newQuotes, - ) - - if (topQuoteData.topAggId) { - topAggId = topQuoteData.topAggId - newQuotes[topAggId].isBestQuote = topQuoteData.isBest - newQuotes[topAggId].savings = topQuoteData.savings - } + const [ + _topAggId, + quotesWithSavingsAndFeeData, + ] = await this._findTopQuoteAndCalculateSavings(newQuotes) + topAggId = _topAggId + newQuotes = quotesWithSavingsAndFeeData } // If a newer call has been made, don't update state with old information @@ -460,15 +457,14 @@ export default class SwapsController { return {} } + const newQuotes = cloneDeep(quotes) + const usedGasPrice = customGasPrice || (await this._getEthersGasPrice()) - let topAggId = '' - let ethTradeValueOfBestQuote = null - let ethFeeForBestQuote = null - const allEthTradeValues = [] - const allEthFees = [] + let topAggId = null + let overallValueOfBestQuoteForSorting = null - Object.values(quotes).forEach((quote) => { + Object.values(newQuotes).forEach((quote) => { const { aggregator, approvalNeeded, @@ -480,6 +476,7 @@ export default class SwapsController { sourceAmount, sourceToken, trade, + fee: metaMaskFee, } = quote const tradeGasLimitForCalculation = gasEstimate @@ -529,60 +526,101 @@ export default class SwapsController { ) : totalEthCost + const decimalAdjustedDestinationAmount = calcTokenAmount( + destinationAmount, + destinationTokenInfo.decimals, + ) + + const tokenPercentageOfPreFeeDestAmount = new BigNumber(100, 10) + .minus(metaMaskFee, 10) + .div(100) + const destinationAmountBeforeMetaMaskFee = decimalAdjustedDestinationAmount.div( + tokenPercentageOfPreFeeDestAmount, + ) + const metaMaskFeeInTokens = destinationAmountBeforeMetaMaskFee.minus( + decimalAdjustedDestinationAmount, + ) + const tokenConversionRate = tokenConversionRates[destinationToken] - const ethValueOfTrade = - destinationToken === ETH_SWAPS_TOKEN_ADDRESS - ? calcTokenAmount(destinationAmount, 18).minus(totalEthCost, 10) - : new BigNumber(tokenConversionRate || 1, 10) - .times( - calcTokenAmount( - destinationAmount, - destinationTokenInfo.decimals, - ), - 10, - ) - .minus(tokenConversionRate ? totalEthCost : 0, 10) - - // collect values for savings calculation - allEthTradeValues.push(ethValueOfTrade) - allEthFees.push(ethFee) + const conversionRateForSorting = tokenConversionRate || 1 + + const ethValueOfTokens = decimalAdjustedDestinationAmount.times( + conversionRateForSorting, + 10, + ) + + const conversionRateForCalculations = + destinationToken === ETH_SWAPS_TOKEN_ADDRESS ? 1 : tokenConversionRate + + const overallValueOfQuoteForSorting = + conversionRateForCalculations === undefined + ? ethValueOfTokens + : ethValueOfTokens.minus(ethFee, 10) + + quote.ethFee = ethFee.toString(10) + + if (conversionRateForCalculations !== undefined) { + quote.ethValueOfTokens = ethValueOfTokens.toString(10) + quote.overallValueOfQuote = overallValueOfQuoteForSorting.toString(10) + quote.metaMaskFeeInEth = metaMaskFeeInTokens + .times(conversionRateForCalculations) + .toString(10) + } if ( - ethTradeValueOfBestQuote === null || - ethValueOfTrade.gt(ethTradeValueOfBestQuote) + overallValueOfBestQuoteForSorting === null || + overallValueOfQuoteForSorting.gt(overallValueOfBestQuoteForSorting) ) { topAggId = aggregator - ethTradeValueOfBestQuote = ethValueOfTrade - ethFeeForBestQuote = ethFee + overallValueOfBestQuoteForSorting = overallValueOfQuoteForSorting } }) const isBest = - quotes[topAggId].destinationToken === ETH_SWAPS_TOKEN_ADDRESS || - Boolean(tokenConversionRates[quotes[topAggId]?.destinationToken]) + newQuotes[topAggId].destinationToken === ETH_SWAPS_TOKEN_ADDRESS || + Boolean(tokenConversionRates[newQuotes[topAggId]?.destinationToken]) let savings = null if (isBest) { + const bestQuote = newQuotes[topAggId] + savings = {} + + const { + ethFee: medianEthFee, + metaMaskFeeInEth: medianMetaMaskFee, + ethValueOfTokens: medianEthValueOfTokens, + } = getMedianEthValueQuote(Object.values(newQuotes)) + // Performance savings are calculated as: - // valueForBestTrade - medianValueOfAllTrades - savings.performance = ethTradeValueOfBestQuote.minus( - getMedian(allEthTradeValues), + // (ethValueOfTokens for the best trade) - (ethValueOfTokens for the media trade) + savings.performance = new BigNumber(bestQuote.ethValueOfTokens, 10).minus( + medianEthValueOfTokens, 10, ) - // Performance savings are calculated as: - // medianFeeOfAllTrades - feeForBestTrade - savings.fee = getMedian(allEthFees).minus(ethFeeForBestQuote, 10) + // Fee savings are calculated as: + // (fee for the median trade) - (fee for the best trade) + savings.fee = new BigNumber(medianEthFee).minus(bestQuote.ethFee, 10) + + savings.metaMaskFee = bestQuote.metaMaskFeeInEth - // Total savings are the sum of performance and fee savings - savings.total = savings.performance.plus(savings.fee, 10).toString(10) + // Total savings are calculated as: + // performance savings + fee savings - metamask fee + savings.total = savings.performance + .plus(savings.fee) + .minus(savings.metaMaskFee) + .toString(10) savings.performance = savings.performance.toString(10) savings.fee = savings.fee.toString(10) + savings.medianMetaMaskFee = medianMetaMaskFee + + newQuotes[topAggId].isBestQuote = true + newQuotes[topAggId].savings = savings } - return { topAggId, isBest, savings } + return [topAggId, newQuotes] } async _getERC20Allowance(contractAddress, walletAddress) { @@ -685,34 +723,123 @@ export default class SwapsController { } /** - * Calculates the median of a sample of BigNumber values. + * Calculates the median overallValueOfQuote of a sample of quotes. * - * @param {import('bignumber.js').BigNumber[]} values - A sample of BigNumber - * values. The array will be sorted in place. - * @returns {import('bignumber.js').BigNumber} The median of the sample. + * @param {Array} quotes - A sample of quote objects with overallValueOfQuote, ethFee, metaMaskFeeInEth, and ethValueOfTokens properties + * @returns {Object} An object with the ethValueOfTokens, ethFee, and metaMaskFeeInEth of the quote with the median overallValueOfQuote */ -function getMedian(values) { - if (!Array.isArray(values) || values.length === 0) { +function getMedianEthValueQuote(_quotes) { + if (!Array.isArray(_quotes) || _quotes.length === 0) { throw new Error('Expected non-empty array param.') } - values.sort((a, b) => { - if (a.equals(b)) { + const quotes = [..._quotes] + + quotes.sort((quoteA, quoteB) => { + const overallValueOfQuoteA = new BigNumber(quoteA.overallValueOfQuote, 10) + const overallValueOfQuoteB = new BigNumber(quoteB.overallValueOfQuote, 10) + if (overallValueOfQuoteA.equals(overallValueOfQuoteB)) { return 0 } - return a.lessThan(b) ? -1 : 1 + return overallValueOfQuoteA.lessThan(overallValueOfQuoteB) ? -1 : 1 }) - if (values.length % 2 === 1) { - // return middle value - return values[(values.length - 1) / 2] + if (quotes.length % 2 === 1) { + // return middle values + const medianOverallValue = + quotes[(quotes.length - 1) / 2].overallValueOfQuote + const quotesMatchingMedianQuoteValue = quotes.filter( + (quote) => medianOverallValue === quote.overallValueOfQuote, + ) + return meansOfQuotesFeesAndValue(quotesMatchingMedianQuoteValue) } // return mean of middle two values - const upperIndex = values.length / 2 - return values[upperIndex].plus(values[upperIndex - 1]).dividedBy(2) + const upperIndex = quotes.length / 2 + const lowerIndex = upperIndex - 1 + + const overallValueAtUpperIndex = quotes[upperIndex].overallValueOfQuote + const overallValueAtLowerIndex = quotes[lowerIndex].overallValueOfQuote + + const quotesMatchingUpperIndexValue = quotes.filter( + (quote) => overallValueAtUpperIndex === quote.overallValueOfQuote, + ) + const quotesMatchingLowerIndexValue = quotes.filter( + (quote) => overallValueAtLowerIndex === quote.overallValueOfQuote, + ) + + const feesAndValueAtUpperIndex = meansOfQuotesFeesAndValue( + quotesMatchingUpperIndexValue, + ) + const feesAndValueAtLowerIndex = meansOfQuotesFeesAndValue( + quotesMatchingLowerIndexValue, + ) + + return { + ethFee: new BigNumber(feesAndValueAtUpperIndex.ethFee, 10) + .plus(feesAndValueAtLowerIndex.ethFee, 10) + .dividedBy(2) + .toString(10), + metaMaskFeeInEth: new BigNumber( + feesAndValueAtUpperIndex.metaMaskFeeInEth, + 10, + ) + .plus(feesAndValueAtLowerIndex.metaMaskFeeInEth, 10) + .dividedBy(2) + .toString(10), + ethValueOfTokens: new BigNumber( + feesAndValueAtUpperIndex.ethValueOfTokens, + 10, + ) + .plus(feesAndValueAtLowerIndex.ethValueOfTokens, 10) + .dividedBy(2) + .toString(10), + } +} + +/** + * Calculates the arithmetic mean for each of three properties - ethFee, metaMaskFeeInEth and ethValueOfTokens - across + * an array of objects containing those properties. + * + * @param {Array} quotes - A sample of quote objects with overallValueOfQuote, ethFee, metaMaskFeeInEth and + * ethValueOfTokens properties + * @returns {Object} An object with the arithmetic mean each of the ethFee, metaMaskFeeInEth and ethValueOfTokens of + * the passed quote objects + */ +function meansOfQuotesFeesAndValue(quotes) { + const feeAndValueSumsAsBigNumbers = quotes.reduce( + (feeAndValueSums, quote) => ({ + ethFee: feeAndValueSums.ethFee.plus(quote.ethFee, 10), + metaMaskFeeInEth: feeAndValueSums.metaMaskFeeInEth.plus( + quote.metaMaskFeeInEth, + 10, + ), + ethValueOfTokens: feeAndValueSums.ethValueOfTokens.plus( + quote.ethValueOfTokens, + 10, + ), + }), + { + ethFee: new BigNumber(0, 10), + metaMaskFeeInEth: new BigNumber(0, 10), + ethValueOfTokens: new BigNumber(0, 10), + }, + ) + + return { + ethFee: feeAndValueSumsAsBigNumbers.ethFee + .div(quotes.length, 10) + .toString(10), + metaMaskFeeInEth: feeAndValueSumsAsBigNumbers.metaMaskFeeInEth + .div(quotes.length, 10) + .toString(10), + ethValueOfTokens: feeAndValueSumsAsBigNumbers.ethValueOfTokens + .div(quotes.length, 10) + .toString(10), + } } export const utils = { - getMedian, + getMedianEthValueQuote, + meansOfQuotesFeesAndValue, } diff --git a/test/unit/app/controllers/swaps-test.js b/test/unit/app/controllers/swaps-test.js index e7f462852..775c53d9f 100644 --- a/test/unit/app/controllers/swaps-test.js +++ b/test/unit/app/controllers/swaps-test.js @@ -2,12 +2,14 @@ import assert from 'assert' import sinon from 'sinon' import { ethers } from 'ethers' +import { mapValues } from 'lodash' import BigNumber from 'bignumber.js' import ObservableStore from 'obs-store' import { ROPSTEN_NETWORK_ID, MAINNET_NETWORK_ID, } from '../../../../app/scripts/controllers/network/enums' +import { ETH_SWAPS_TOKEN_ADDRESS } from '../../../../ui/app/helpers/constants/swaps' import { createTestProviderTools } from '../../../stub/provider' import SwapsController, { utils, @@ -25,6 +27,10 @@ const MOCK_FETCH_PARAMS = { const TEST_AGG_ID_1 = 'TEST_AGG_1' const TEST_AGG_ID_2 = 'TEST_AGG_2' +const TEST_AGG_ID_3 = 'TEST_AGG_3' +const TEST_AGG_ID_4 = 'TEST_AGG_4' +const TEST_AGG_ID_5 = 'TEST_AGG_5' +const TEST_AGG_ID_6 = 'TEST_AGG_6' const TEST_AGG_ID_BEST = 'TEST_AGG_BEST' const TEST_AGG_ID_APPROVAL = 'TEST_AGG_APPROVAL' @@ -61,6 +67,7 @@ const MOCK_QUOTES_APPROVAL_REQUIRED = { aggType: 'AGG', slippage: 3, approvalNeeded: MOCK_APPROVAL_NEEDED, + fee: 1, }, } @@ -72,7 +79,10 @@ const MOCK_FETCH_METADATA = { } const MOCK_TOKEN_RATES_STORE = new ObservableStore({ - contractExchangeRates: { '0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48': 2 }, + contractExchangeRates: { + '0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48': 2, + '0x1111111111111111111111111111111111111111': 0.1, + }, }) const MOCK_GET_PROVIDER_CONFIG = () => ({ type: 'FAKE_NETWORK' }) @@ -350,6 +360,13 @@ describe('SwapsController', function () { }) describe('_findTopQuoteAndCalculateSavings', function () { + beforeEach(function () { + const { swapsState } = swapsController.store.getState() + swapsController.store.updateState({ + swapsState: { ...swapsState, customGasPrice: '0x174876e800' }, + }) + }) + it('returns empty object if passed undefined or empty object', async function () { assert.deepStrictEqual( await swapsController._findTopQuoteAndCalculateSavings(), @@ -360,6 +377,261 @@ describe('SwapsController', function () { {}, ) }) + + it('returns the top aggId and quotes with savings and fee values if passed necessary data and an even number of quotes', async function () { + const [ + topAggId, + resultQuotes, + ] = await swapsController._findTopQuoteAndCalculateSavings( + getTopQuoteAndSavingsMockQuotes(), + ) + assert.equal(topAggId, TEST_AGG_ID_1) + assert.deepStrictEqual( + resultQuotes, + getTopQuoteAndSavingsBaseExpectedResults(), + ) + }) + + it('returns the top aggId and quotes with savings and fee values if passed necessary data and an odd number of quotes', async function () { + const testInput = getTopQuoteAndSavingsMockQuotes() + delete testInput[TEST_AGG_ID_6] + const expectedResultQuotes = getTopQuoteAndSavingsBaseExpectedResults() + delete expectedResultQuotes[TEST_AGG_ID_6] + expectedResultQuotes[TEST_AGG_ID_1].savings = { + total: '0.0292', + performance: '0.0297', + fee: '0.02', + metaMaskFee: '0.0205', + medianMetaMaskFee: '0.0202', + } + + const [ + topAggId, + resultQuotes, + ] = await swapsController._findTopQuoteAndCalculateSavings(testInput) + assert.equal(topAggId, TEST_AGG_ID_1) + assert.deepStrictEqual(resultQuotes, expectedResultQuotes) + }) + + it('returns the top aggId, without best quote flagged, and quotes with fee values if passed necessary data but no custom convert rate exists', async function () { + const testInput = mapValues( + getTopQuoteAndSavingsMockQuotes(), + (quote) => ({ + ...quote, + destinationToken: '0xnoConversionRateExists', + }), + ) + const expectedResultQuotes = { + [TEST_AGG_ID_1]: { + ...testInput[TEST_AGG_ID_1], + ethFee: '0.01', + }, + [TEST_AGG_ID_2]: { + ...testInput[TEST_AGG_ID_2], + ethFee: '0.02', + }, + [TEST_AGG_ID_3]: { + ...testInput[TEST_AGG_ID_3], + ethFee: '0.03', + }, + [TEST_AGG_ID_4]: { + ...testInput[TEST_AGG_ID_4], + ethFee: '0.04', + }, + [TEST_AGG_ID_5]: { + ...testInput[TEST_AGG_ID_5], + ethFee: '0.05', + }, + [TEST_AGG_ID_6]: { + ...testInput[TEST_AGG_ID_6], + ethFee: '0.06', + }, + } + + const [ + topAggId, + resultQuotes, + ] = await swapsController._findTopQuoteAndCalculateSavings(testInput) + assert.equal(topAggId, TEST_AGG_ID_1) + assert.deepStrictEqual(resultQuotes, expectedResultQuotes) + }) + + it('returns the top aggId and quotes with savings and fee values if passed necessary data and the source token is ETH', async function () { + const testInput = mapValues( + getTopQuoteAndSavingsMockQuotes(), + (quote) => ({ + ...quote, + sourceToken: ETH_SWAPS_TOKEN_ADDRESS, + destinationToken: '0x1111111111111111111111111111111111111111', + trade: { value: '0x8ac7230489e80000' }, + }), + ) + const baseExpectedResultQuotes = getTopQuoteAndSavingsBaseExpectedResults() + const expectedResultQuotes = { + [TEST_AGG_ID_1]: { + ...baseExpectedResultQuotes[TEST_AGG_ID_1], + sourceToken: ETH_SWAPS_TOKEN_ADDRESS, + destinationToken: '0x1111111111111111111111111111111111111111', + trade: { value: '0x8ac7230489e80000' }, + overallValueOfQuote: '2.0195', + }, + [TEST_AGG_ID_2]: { + ...baseExpectedResultQuotes[TEST_AGG_ID_2], + sourceToken: ETH_SWAPS_TOKEN_ADDRESS, + destinationToken: '0x1111111111111111111111111111111111111111', + trade: { value: '0x8ac7230489e80000' }, + overallValueOfQuote: '1.9996', + }, + [TEST_AGG_ID_3]: { + ...baseExpectedResultQuotes[TEST_AGG_ID_3], + sourceToken: ETH_SWAPS_TOKEN_ADDRESS, + destinationToken: '0x1111111111111111111111111111111111111111', + trade: { value: '0x8ac7230489e80000' }, + overallValueOfQuote: '1.9698', + }, + [TEST_AGG_ID_4]: { + ...baseExpectedResultQuotes[TEST_AGG_ID_4], + sourceToken: ETH_SWAPS_TOKEN_ADDRESS, + destinationToken: '0x1111111111111111111111111111111111111111', + trade: { value: '0x8ac7230489e80000' }, + overallValueOfQuote: '1.94', + }, + [TEST_AGG_ID_5]: { + ...baseExpectedResultQuotes[TEST_AGG_ID_5], + sourceToken: ETH_SWAPS_TOKEN_ADDRESS, + destinationToken: '0x1111111111111111111111111111111111111111', + trade: { value: '0x8ac7230489e80000' }, + overallValueOfQuote: '1.9102', + }, + [TEST_AGG_ID_6]: { + ...baseExpectedResultQuotes[TEST_AGG_ID_6], + sourceToken: ETH_SWAPS_TOKEN_ADDRESS, + destinationToken: '0x1111111111111111111111111111111111111111', + trade: { value: '0x8ac7230489e80000' }, + overallValueOfQuote: '1.8705', + }, + } + + const [ + topAggId, + resultQuotes, + ] = await swapsController._findTopQuoteAndCalculateSavings(testInput) + assert.equal(topAggId, TEST_AGG_ID_1) + assert.deepStrictEqual(resultQuotes, expectedResultQuotes) + }) + + it('returns the top aggId and quotes with savings and fee values if passed necessary data and the source token is ETH and an ETH fee is included in the trade value of what would be the best quote', async function () { + const testInput = mapValues( + getTopQuoteAndSavingsMockQuotes(), + (quote) => ({ + ...quote, + sourceToken: ETH_SWAPS_TOKEN_ADDRESS, + destinationToken: '0x1111111111111111111111111111111111111111', + trade: { value: '0x8ac7230489e80000' }, + }), + ) + // 0.04 ETH fee included in trade value + testInput[TEST_AGG_ID_1].trade.value = '0x8b553ece48ec0000' + const baseExpectedResultQuotes = getTopQuoteAndSavingsBaseExpectedResults() + const expectedResultQuotes = { + [TEST_AGG_ID_1]: { + ...baseExpectedResultQuotes[TEST_AGG_ID_1], + sourceToken: ETH_SWAPS_TOKEN_ADDRESS, + destinationToken: '0x1111111111111111111111111111111111111111', + trade: { value: '0x8b553ece48ec0000' }, + overallValueOfQuote: '1.9795', + ethFee: '0.05', + }, + [TEST_AGG_ID_2]: { + ...baseExpectedResultQuotes[TEST_AGG_ID_2], + sourceToken: ETH_SWAPS_TOKEN_ADDRESS, + destinationToken: '0x1111111111111111111111111111111111111111', + trade: { value: '0x8ac7230489e80000' }, + overallValueOfQuote: '1.9996', + isBestQuote: true, + savings: { + total: '0.0243', + performance: '0.0297', + fee: '0.015', + metaMaskFee: '0.0204', + medianMetaMaskFee: '0.0201', + }, + }, + [TEST_AGG_ID_3]: { + ...baseExpectedResultQuotes[TEST_AGG_ID_3], + sourceToken: ETH_SWAPS_TOKEN_ADDRESS, + destinationToken: '0x1111111111111111111111111111111111111111', + trade: { value: '0x8ac7230489e80000' }, + overallValueOfQuote: '1.9698', + }, + [TEST_AGG_ID_4]: { + ...baseExpectedResultQuotes[TEST_AGG_ID_4], + sourceToken: ETH_SWAPS_TOKEN_ADDRESS, + destinationToken: '0x1111111111111111111111111111111111111111', + trade: { value: '0x8ac7230489e80000' }, + overallValueOfQuote: '1.94', + }, + [TEST_AGG_ID_5]: { + ...baseExpectedResultQuotes[TEST_AGG_ID_5], + sourceToken: ETH_SWAPS_TOKEN_ADDRESS, + destinationToken: '0x1111111111111111111111111111111111111111', + trade: { value: '0x8ac7230489e80000' }, + overallValueOfQuote: '1.9102', + }, + [TEST_AGG_ID_6]: { + ...baseExpectedResultQuotes[TEST_AGG_ID_6], + sourceToken: ETH_SWAPS_TOKEN_ADDRESS, + destinationToken: '0x1111111111111111111111111111111111111111', + trade: { value: '0x8ac7230489e80000' }, + overallValueOfQuote: '1.8705', + }, + } + delete expectedResultQuotes[TEST_AGG_ID_1].isBestQuote + delete expectedResultQuotes[TEST_AGG_ID_1].savings + + const [ + topAggId, + resultQuotes, + ] = await swapsController._findTopQuoteAndCalculateSavings(testInput) + assert.equal(topAggId, TEST_AGG_ID_2) + assert.deepStrictEqual(resultQuotes, expectedResultQuotes) + }) + + it('returns the top aggId and quotes with savings and fee values if passed necessary data and the source token is not ETH and an ETH fee is included in the trade value of what would be the best quote', async function () { + const testInput = getTopQuoteAndSavingsMockQuotes() + // 0.04 ETH fee included in trade value + testInput[TEST_AGG_ID_1].trade.value = '0x8e1bc9bf040000' + const baseExpectedResultQuotes = getTopQuoteAndSavingsBaseExpectedResults() + const expectedResultQuotes = { + ...baseExpectedResultQuotes, + [TEST_AGG_ID_1]: { + ...baseExpectedResultQuotes[TEST_AGG_ID_1], + trade: { value: '0x8e1bc9bf040000' }, + overallValueOfQuote: '1.9795', + ethFee: '0.05', + }, + [TEST_AGG_ID_2]: { + ...baseExpectedResultQuotes[TEST_AGG_ID_2], + isBestQuote: true, + savings: { + total: '0.0243', + performance: '0.0297', + fee: '0.015', + metaMaskFee: '0.0204', + medianMetaMaskFee: '0.0201', + }, + }, + } + delete expectedResultQuotes[TEST_AGG_ID_1].isBestQuote + delete expectedResultQuotes[TEST_AGG_ID_1].savings + + const [ + topAggId, + resultQuotes, + ] = await swapsController._findTopQuoteAndCalculateSavings(testInput) + assert.equal(topAggId, [TEST_AGG_ID_2]) + assert.deepStrictEqual(resultQuotes, expectedResultQuotes) + }) }) describe('fetchAndSetQuotes', function () { @@ -394,9 +666,15 @@ describe('SwapsController', function () { gasEstimateWithRefund: 'b8cae', savings: { fee: '0', + metaMaskFee: '0.5050505050505050505', performance: '6', - total: '6', + total: '5.4949494949494949495', + medianMetaMaskFee: '0.44444444444444444444', }, + ethFee: '33554432', + overallValueOfQuote: '-33554382', + metaMaskFeeInEth: '0.5050505050505050505', + ethValueOfTokens: '50', }) assert.strictEqual( @@ -515,7 +793,7 @@ describe('SwapsController', function () { MOCK_FETCH_METADATA, ) - assert.strictEqual(newQuotes[topAggId].isBestQuote, false) + assert.strictEqual(newQuotes[topAggId].isBestQuote, undefined) }) }) @@ -865,37 +1143,215 @@ describe('SwapsController', function () { }) describe('utils', function () { - describe('getMedian', function () { - const { getMedian } = utils + describe('getMedianEthValueQuote', function () { + const { getMedianEthValueQuote } = utils it('calculates median correctly with uneven sample', function () { - const values = [3, 2, 6].map((value) => new BigNumber(value)) - const median = getMedian(values) + const expectedResult = { + ethFee: '10', + metaMaskFeeInEth: '5', + ethValueOfTokens: '0.3', + } + const values = [ + { + overallValueOfQuote: '3', + ethFee: '10', + metaMaskFeeInEth: '5', + ethValueOfTokens: '0.3', + }, + { + overallValueOfQuote: '2', + ethFee: '20', + metaMaskFeeInEth: '3', + ethValueOfTokens: '0.2', + }, + { + overallValueOfQuote: '6', + ethFee: '40', + metaMaskFeeInEth: '6', + ethValueOfTokens: '0.6', + }, + ] - assert.strictEqual( - median.toNumber(), - 3, - 'should have returned correct median', + const median = getMedianEthValueQuote(values) + + assert.deepEqual( + median, + expectedResult, + 'should have returned correct median quote object', ) }) it('calculates median correctly with even sample', function () { - const values = [3, 2, 2, 6].map((value) => new BigNumber(value)) - const median = getMedian(values) + const expectedResult = { + ethFee: '20', + metaMaskFeeInEth: '6.5', + ethValueOfTokens: '0.25', + } + const values = [ + { + overallValueOfQuote: '3', + ethFee: '10', + metaMaskFeeInEth: '5', + ethValueOfTokens: '0.3', + }, + { + overallValueOfQuote: '1', + ethFee: '20', + metaMaskFeeInEth: '3', + ethValueOfTokens: '0.2', + }, + { + overallValueOfQuote: '2', + ethFee: '30', + metaMaskFeeInEth: '8', + ethValueOfTokens: '0.2', + }, + { + overallValueOfQuote: '6', + ethFee: '40', + metaMaskFeeInEth: '6', + ethValueOfTokens: '0.6', + }, + ] + const median = getMedianEthValueQuote(values) - assert.strictEqual( - median.toNumber(), - 2.5, - 'should have returned correct median', + assert.deepEqual( + median, + expectedResult, + 'should have returned correct median quote object', + ) + }) + + it('calculates median correctly with an uneven sample where multiple quotes have the median overall value', function () { + const expectedResult = { + ethFee: '2', + metaMaskFeeInEth: '0.5', + ethValueOfTokens: '5', + } + + const values = [ + { + overallValueOfQuote: '1', + ethValueOfTokens: '2', + ethFee: '1', + metaMaskFeeInEth: '0.2', + }, + { + overallValueOfQuote: '3', + ethValueOfTokens: '4', + ethFee: '1', + metaMaskFeeInEth: '0.4', + }, + { + overallValueOfQuote: '3', + ethValueOfTokens: '5', + ethFee: '2', + metaMaskFeeInEth: '0.5', + }, + { + overallValueOfQuote: '3', + ethValueOfTokens: '6', + ethFee: '3', + metaMaskFeeInEth: '0.6', + }, + { + overallValueOfQuote: '4', + ethValueOfTokens: '6', + ethFee: '2', + metaMaskFeeInEth: '0.6', + }, + { + overallValueOfQuote: '4', + ethValueOfTokens: '7', + ethFee: '3', + metaMaskFeeInEth: '0.7', + }, + { + overallValueOfQuote: '6', + ethValueOfTokens: '8', + ethFee: '2', + metaMaskFeeInEth: '0.8', + }, + ] + const median = getMedianEthValueQuote(values) + + assert.deepEqual( + median, + expectedResult, + 'should have returned correct median quote object', + ) + }) + + it('calculates median correctly with an even sample where multiple quotes have the same overall value as either of the two middle values', function () { + const expectedResult = { + ethFee: '2', + metaMaskFeeInEth: '0.55', + ethValueOfTokens: '5.5', + } + + const values = [ + { + overallValueOfQuote: '1', + ethValueOfTokens: '2', + ethFee: '1', + metaMaskFeeInEth: '0.2', + }, + { + overallValueOfQuote: '3', + ethValueOfTokens: '4', + ethFee: '1', + metaMaskFeeInEth: '0.4', + }, + { + overallValueOfQuote: '3', + ethValueOfTokens: '5', + ethFee: '2', + metaMaskFeeInEth: '0.5', + }, + { + overallValueOfQuote: '4', + ethValueOfTokens: '6', + ethFee: '2', + metaMaskFeeInEth: '0.6', + }, + { + overallValueOfQuote: '4', + ethValueOfTokens: '7', + ethFee: '3', + metaMaskFeeInEth: '0.7', + }, + { + overallValueOfQuote: '6', + ethValueOfTokens: '8', + ethFee: '2', + metaMaskFeeInEth: '0.8', + }, + ] + const median = getMedianEthValueQuote(values) + + assert.deepEqual( + median, + expectedResult, + 'should have returned correct median quote object', ) }) it('throws on empty or non-array sample', function () { - assert.throws(() => getMedian([]), 'should throw on empty array') + assert.throws( + () => getMedianEthValueQuote([]), + 'should throw on empty array', + ) - assert.throws(() => getMedian(), 'should throw on non-array param') + assert.throws( + () => getMedianEthValueQuote(), + 'should throw on non-array param', + ) - assert.throws(() => getMedian({}), 'should throw on non-array param') + assert.throws( + () => getMedianEthValueQuote({}), + 'should throw on non-array param', + ) }) }) }) @@ -934,6 +1390,7 @@ function getMockQuotes() { symbol: 'USDC', decimals: 18, }, + fee: 1, }, [TEST_AGG_ID_BEST]: { @@ -967,6 +1424,7 @@ function getMockQuotes() { symbol: 'USDC', decimals: 18, }, + fee: 1, }, [TEST_AGG_ID_2]: { @@ -1000,6 +1458,160 @@ function getMockQuotes() { symbol: 'USDC', decimals: 18, }, + fee: 1, + }, + } +} + +function getTopQuoteAndSavingsMockQuotes() { + // These destination amounts are calculated using the following "pre-fee" amounts + // TEST_AGG_ID_1: 20.5 + // TEST_AGG_ID_2: 20.4 + // TEST_AGG_ID_3: 20.2 + // TEST_AGG_ID_4: 20 + // TEST_AGG_ID_5: 19.8 + // TEST_AGG_ID_6: 19.5 + + return { + [TEST_AGG_ID_1]: { + aggregator: TEST_AGG_ID_1, + approvalNeeded: null, + gasEstimate: '0x186a0', + destinationAmount: '20295000000000000000', + destinationToken: '0x1111111111111111111111111111111111111111', + destinationTokenInfo: { decimals: 18 }, + sourceAmount: '10000000000000000000', + sourceToken: '0xsomeERC20TokenAddress', + trade: { + value: '0x0', + }, + fee: 1, + }, + [TEST_AGG_ID_2]: { + aggregator: TEST_AGG_ID_2, + approvalNeeded: null, + gasEstimate: '0x30d40', + destinationAmount: '20196000000000000000', + destinationToken: '0x1111111111111111111111111111111111111111', + destinationTokenInfo: { decimals: 18 }, + sourceAmount: '10000000000000000000', + sourceToken: '0xsomeERC20TokenAddress', + trade: { + value: '0x0', + }, + fee: 1, + }, + [TEST_AGG_ID_3]: { + aggregator: TEST_AGG_ID_3, + approvalNeeded: null, + gasEstimate: '0x493e0', + destinationAmount: '19998000000000000000', + destinationToken: '0x1111111111111111111111111111111111111111', + destinationTokenInfo: { decimals: 18 }, + sourceAmount: '10000000000000000000', + sourceToken: '0xsomeERC20TokenAddress', + trade: { + value: '0x0', + }, + fee: 1, + }, + [TEST_AGG_ID_4]: { + aggregator: TEST_AGG_ID_4, + approvalNeeded: null, + gasEstimate: '0x61a80', + destinationAmount: '19800000000000000000', + destinationToken: '0x1111111111111111111111111111111111111111', + destinationTokenInfo: { decimals: 18 }, + sourceAmount: '10000000000000000000', + sourceToken: '0xsomeERC20TokenAddress', + trade: { + value: '0x0', + }, + fee: 1, + }, + [TEST_AGG_ID_5]: { + aggregator: TEST_AGG_ID_5, + approvalNeeded: null, + gasEstimate: '0x7a120', + destinationAmount: '19602000000000000000', + destinationToken: '0x1111111111111111111111111111111111111111', + destinationTokenInfo: { decimals: 18 }, + sourceAmount: '10000000000000000000', + sourceToken: '0xsomeERC20TokenAddress', + trade: { + value: '0x0', + }, + fee: 1, + }, + [TEST_AGG_ID_6]: { + aggregator: TEST_AGG_ID_6, + approvalNeeded: null, + gasEstimate: '0x927c0', + destinationAmount: '19305000000000000000', + destinationToken: '0x1111111111111111111111111111111111111111', + destinationTokenInfo: { decimals: 18 }, + sourceAmount: '10000000000000000000', + sourceToken: '0xsomeERC20TokenAddress', + trade: { + value: '0x0', + }, + fee: 1, + }, + } +} + +function getTopQuoteAndSavingsBaseExpectedResults() { + const baseTestInput = getTopQuoteAndSavingsMockQuotes() + return { + [TEST_AGG_ID_1]: { + ...baseTestInput[TEST_AGG_ID_1], + isBestQuote: true, + ethFee: '0.01', + overallValueOfQuote: '2.0195', + metaMaskFeeInEth: '0.0205', + ethValueOfTokens: '2.0295', + savings: { + total: '0.0441', + performance: '0.0396', + fee: '0.025', + metaMaskFee: '0.0205', + medianMetaMaskFee: '0.0201', + }, + }, + [TEST_AGG_ID_2]: { + ...baseTestInput[TEST_AGG_ID_2], + ethFee: '0.02', + overallValueOfQuote: '1.9996', + metaMaskFeeInEth: '0.0204', + ethValueOfTokens: '2.0196', + }, + [TEST_AGG_ID_3]: { + ...baseTestInput[TEST_AGG_ID_3], + ethFee: '0.03', + overallValueOfQuote: '1.9698', + metaMaskFeeInEth: '0.0202', + ethValueOfTokens: '1.9998', + }, + [TEST_AGG_ID_4]: { + ...baseTestInput[TEST_AGG_ID_4], + ethFee: '0.04', + overallValueOfQuote: '1.94', + metaMaskFeeInEth: '0.02', + ethValueOfTokens: '1.98', + }, + [TEST_AGG_ID_5]: { + ...baseTestInput[TEST_AGG_ID_5], + ethFee: '0.05', + overallValueOfQuote: '1.9102', + metaMaskFeeInEth: '0.0198', + ethValueOfTokens: '1.9602', + }, + [TEST_AGG_ID_6]: { + ...baseTestInput[TEST_AGG_ID_6], + ethFee: '0.06', + overallValueOfQuote: '1.8705', + metaMaskFeeInEth: '0.0195', + ethValueOfTokens: '1.9305', }, } }