diff --git a/app/scripts/controllers/swaps.js b/app/scripts/controllers/swaps.js index 682eff4aa..ced804a35 100644 --- a/app/scripts/controllers/swaps.js +++ b/app/scripts/controllers/swaps.js @@ -175,11 +175,12 @@ export default class SwapsController { if (Object.values(newQuotes).length === 0) { this.setSwapsErrorKey(QUOTES_NOT_AVAILABLE_ERROR) } else { - const topAggData = await this._findTopQuoteAggId(newQuotes) + const topQuoteData = await this._findTopQuoteAndCalculateSavings(newQuotes) - if (topAggData.topAggId) { - topAggId = topAggData.topAggId - newQuotes[topAggId].isBestQuote = topAggData.isBest + if (topQuoteData.topAggId) { + topAggId = topQuoteData.topAggId + newQuotes[topAggId].isBestQuote = topQuoteData.isBest + newQuotes[topAggId].savings = topQuoteData.savings } } @@ -394,48 +395,61 @@ export default class SwapsController { return ethersGasPrice.toHexString() } - async _findTopQuoteAggId (quotes) { + async _findTopQuoteAndCalculateSavings (quotes = {}) { const tokenConversionRates = this.tokenRatesStore.getState() .contractExchangeRates const { swapsState: { customGasPrice }, } = this.store.getState() - if (!Object.values(quotes).length) { + const numQuotes = Object.keys(quotes).length + if (!numQuotes) { return {} } const usedGasPrice = customGasPrice || await this._getEthersGasPrice() let topAggId = '' - let ethValueOfTradeForBestQuote = null + let ethTradeValueOfBestQuote = null + let ethFeeForBestQuote = null + const allEthTradeValues = [] + const allEthFees = [] Object.values(quotes).forEach((quote) => { const { + aggregator, + approvalNeeded, + averageGas, destinationAmount = 0, destinationToken, destinationTokenInfo, - trade, - approvalNeeded, - averageGas, gasEstimate, - aggregator, + sourceAmount, + sourceToken, + trade, } = quote + const tradeGasLimitForCalculation = gasEstimate ? new BigNumber(gasEstimate, 16) : new BigNumber(averageGas || MAX_GAS_LIMIT, 10) + const totalGasLimitForCalculation = tradeGasLimitForCalculation .plus(approvalNeeded?.gas || '0x0', 16) .toString(16) + const gasTotalInWeiHex = calcGasTotal( totalGasLimitForCalculation, usedGasPrice, ) - const totalEthCost = new BigNumber(gasTotalInWeiHex, 16).plus( - trade.value, - 16, - ) - const ethFee = conversionUtil(totalEthCost, { + + // trade.value is a sum of different values depending on the transaction. + // It always includes any external fees charged by the quote source. In + // addition, if the source asset is ETH, trade.value includes the amount + // of swapped ETH. + const totalWeiCost = new BigNumber(gasTotalInWeiHex, 16) + .plus(trade.value, 16) + + const totalEthCost = conversionUtil(totalWeiCost, { fromCurrency: 'ETH', fromDenomination: 'WEI', toDenomination: 'ETH', @@ -443,10 +457,26 @@ export default class SwapsController { numberOfDecimals: 6, }) + // The total fee is aggregator/exchange fees plus gas fees. + // If the swap is from ETH, subtract the sourceAmount from the total cost. + // Otherwise, the total fee is simply trade.value plus gas fees. + const ethFee = sourceToken === ETH_SWAPS_TOKEN_ADDRESS + ? conversionUtil( + totalWeiCost.minus(sourceAmount, 10), // sourceAmount is in wei + { + fromCurrency: 'ETH', + fromDenomination: 'WEI', + toDenomination: 'ETH', + fromNumericBase: 'BN', + numberOfDecimals: 6, + }, + ) + : totalEthCost + const tokenConversionRate = tokenConversionRates[destinationToken] const ethValueOfTrade = - destinationTokenInfo.symbol === 'ETH' - ? calcTokenAmount(destinationAmount, 18).minus(ethFee, 10) + destinationToken === ETH_SWAPS_TOKEN_ADDRESS + ? calcTokenAmount(destinationAmount, 18).minus(totalEthCost, 10) : new BigNumber(tokenConversionRate || 1, 10) .times( calcTokenAmount( @@ -455,22 +485,51 @@ export default class SwapsController { ), 10, ) - .minus(tokenConversionRate ? ethFee.toString(10) : 0, 10) + .minus(tokenConversionRate ? totalEthCost : 0, 10) + + // collect values for savings calculation + allEthTradeValues.push(ethValueOfTrade) + allEthFees.push(ethFee) if ( - ethValueOfTradeForBestQuote === null || - ethValueOfTrade.gt(ethValueOfTradeForBestQuote) + ethTradeValueOfBestQuote === null || + ethValueOfTrade.gt(ethTradeValueOfBestQuote) ) { topAggId = aggregator - ethValueOfTradeForBestQuote = ethValueOfTrade + ethTradeValueOfBestQuote = ethValueOfTrade + ethFeeForBestQuote = ethFee } }) const isBest = - quotes[topAggId]?.destinationTokenInfo?.symbol === 'ETH' || + quotes[topAggId].destinationToken === ETH_SWAPS_TOKEN_ADDRESS || Boolean(tokenConversionRates[quotes[topAggId]?.destinationToken]) - return { topAggId, isBest } + let savings = null + + if (isBest) { + savings = {} + // Performance savings are calculated as: + // valueForBestTrade - medianValueOfAllTrades + savings.performance = ethTradeValueOfBestQuote.minus( + getMedian(allEthTradeValues), + 10, + ) + + // Performance savings are calculated as: + // medianFeeOfAllTrades - feeForBestTrade + savings.fee = getMedian(allEthFees).minus( + ethFeeForBestQuote, + 10, + ) + + // Total savings are the sum of performance and fee savings + savings.total = savings.performance.plus(savings.fee, 10).toString(10) + savings.performance = savings.performance.toString(10) + savings.fee = savings.fee.toString(10) + } + + return { topAggId, isBest, savings } } async _getERC20Allowance (contractAddress, walletAddress) { @@ -563,5 +622,39 @@ export default class SwapsController { this.setSwapsLiveness(swapsFeatureIsLive) } } +} + +/** + * Calculates the median of a sample of BigNumber values. + * + * @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. + */ +function getMedian (values) { + if (!Array.isArray(values) || values.length === 0) { + throw new Error('Expected non-empty array param.') + } + + values.sort((a, b) => { + if (a.equals(b)) { + return 0 + } + return a.lessThan(b) ? -1 : 1 + }) + + if (values.length % 2 === 1) { + // return middle value + return values[(values.length - 1) / 2] + } + + // return mean of middle two values + const upperIndex = values.length / 2 + return values[upperIndex] + .plus(values[upperIndex - 1]) + .dividedBy(2) +} +export const utils = { + getMedian, } diff --git a/test/unit/app/controllers/swaps-test.js b/test/unit/app/controllers/swaps-test.js index 60395403c..ff54a3fd2 100644 --- a/test/unit/app/controllers/swaps-test.js +++ b/test/unit/app/controllers/swaps-test.js @@ -5,7 +5,7 @@ import { ethers } from 'ethers' import BigNumber from 'bignumber.js' import ObservableStore from 'obs-store' import { createTestProviderTools } from '../../../stub/provider' -import SwapsController from '../../../../app/scripts/controllers/swaps' +import SwapsController, { utils } from '../../../../app/scripts/controllers/swaps' const MOCK_FETCH_PARAMS = { slippage: 3, @@ -17,32 +17,11 @@ const MOCK_FETCH_PARAMS = { exchangeList: 'zeroExV1', } -const TEST_AGG_ID = 'zeroExV1' -const MOCK_QUOTES = { - [TEST_AGG_ID]: { - trade: { - data: '0x00', - from: '0x7F18BB4Dd92CF2404C54CBa1A9BE4A1153bdb078', - value: '0x17647444f166000', - gas: '0xe09c0', - gasPrice: undefined, - to: '0x881d40237659c251811cec9c364ef91dc08d300c', - }, - sourceAmount: '1000000000000000000000000000000000000', - destinationAmount: '396493201125465', - error: null, - sourceToken: '0x6b175474e89094c44da98b954eedeac495271d0f', - destinationToken: '0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48', - approvalNeeded: null, - maxGas: 920000, - averageGas: 312510, - estimatedRefund: 343090, - fetchTime: 559, - aggregator: TEST_AGG_ID, - aggType: 'AGG', - slippage: 3, - }, -} +const TEST_AGG_ID_1 = 'TEST_AGG_1' +const TEST_AGG_ID_2 = 'TEST_AGG_2' +const TEST_AGG_ID_BEST = 'TEST_AGG_BEST' +const TEST_AGG_ID_APPROVAL = 'TEST_AGG_APPROVAL' + const MOCK_APPROVAL_NEEDED = { 'data': '0x095ea7b300000000000000000000000095e6f48254609a6ee006f7d493c8e5fb97094cef0000000000000000000000000000000000000000004a817c7ffffffdabf41c00', 'to': '0xc02aaa39b223fe8d0a0e5c4f27ead9083c756cc2', @@ -51,8 +30,9 @@ const MOCK_APPROVAL_NEEDED = { 'gas': '12', 'gasPrice': '34', } + const MOCK_QUOTES_APPROVAL_REQUIRED = { - [TEST_AGG_ID]: { + [TEST_AGG_ID_APPROVAL]: { trade: { data: '0x00', from: '0x7F18BB4Dd92CF2404C54CBa1A9BE4A1153bdb078', @@ -70,12 +50,13 @@ const MOCK_QUOTES_APPROVAL_REQUIRED = { averageGas: 312510, estimatedRefund: 343090, fetchTime: 559, - aggregator: TEST_AGG_ID, + aggregator: TEST_AGG_ID_APPROVAL, aggType: 'AGG', slippage: 3, approvalNeeded: MOCK_APPROVAL_NEEDED, }, } + const MOCK_FETCH_METADATA = { destinationTokenInfo: { symbol: 'FOO', @@ -233,14 +214,14 @@ describe('SwapsController', function () { }) it('should set initial gas estimate', async function () { - const initialAggId = TEST_AGG_ID + const initialAggId = TEST_AGG_ID_1 const baseGasEstimate = 10 - const { maxGas, estimatedRefund } = MOCK_QUOTES[TEST_AGG_ID] + const { maxGas, estimatedRefund } = getMockQuotes()[TEST_AGG_ID_1] const { swapsState } = swapsController.store.getState() // Set mock quotes in order to have data for the test agg swapsController.store.updateState({ - swapsState: { ...swapsState, quotes: MOCK_QUOTES }, + swapsState: { ...swapsState, quotes: getMockQuotes() }, }) await swapsController.setInitialGasEstimate( @@ -272,6 +253,19 @@ describe('SwapsController', function () { }) }) + describe('_findTopQuoteAndCalculateSavings', function () { + it('returns empty object if passed undefined or empty object', async function () { + assert.deepStrictEqual( + await swapsController._findTopQuoteAndCalculateSavings(), + {}, + ) + assert.deepStrictEqual( + await swapsController._findTopQuoteAndCalculateSavings({}), + {}, + ) + }) + }) + describe('fetchAndSetQuotes', function () { it('returns null if fetchParams is not provided', async function () { const quotes = await swapsController.fetchAndSetQuotes(undefined) @@ -279,7 +273,7 @@ describe('SwapsController', function () { }) it('calls fetchTradesInfo with the given fetchParams and returns the correct quotes', async function () { - fetchTradesInfoStub.resolves(MOCK_QUOTES) + fetchTradesInfoStub.resolves(getMockQuotes()) // Make it so approval is not required sandbox @@ -291,8 +285,8 @@ describe('SwapsController', function () { MOCK_FETCH_METADATA, ) - assert.deepStrictEqual(newQuotes[TEST_AGG_ID], { - ...MOCK_QUOTES[TEST_AGG_ID], + assert.deepStrictEqual(newQuotes[TEST_AGG_ID_BEST], { + ...getMockQuotes()[TEST_AGG_ID_BEST], sourceTokenInfo: undefined, destinationTokenInfo: { symbol: 'FOO', @@ -301,7 +295,12 @@ describe('SwapsController', function () { isBestQuote: true, // TODO: find a way to calculate these values dynamically gasEstimate: 2000000, - gasEstimateWithRefund: '8cd8e', + gasEstimateWithRefund: 'b8cae', + savings: { + fee: '0', + performance: '6', + total: '6', + }, }) assert.strictEqual( @@ -311,7 +310,7 @@ describe('SwapsController', function () { }) it('performs the allowance check', async function () { - fetchTradesInfoStub.resolves(MOCK_QUOTES) + fetchTradesInfoStub.resolves(getMockQuotes()) // Make it so approval is not required const allowanceStub = sandbox @@ -358,7 +357,7 @@ describe('SwapsController', function () { }) it('marks the best quote', async function () { - fetchTradesInfoStub.resolves(MOCK_QUOTES) + fetchTradesInfoStub.resolves(getMockQuotes()) // Make it so approval is not required sandbox @@ -370,7 +369,7 @@ describe('SwapsController', function () { MOCK_FETCH_METADATA, ) - assert.strictEqual(topAggId, TEST_AGG_ID) + assert.strictEqual(topAggId, TEST_AGG_ID_BEST) assert.strictEqual(newQuotes[topAggId].isBestQuote, true) }) @@ -379,15 +378,15 @@ describe('SwapsController', function () { // Clone the existing mock quote and increase destination amount const bestQuote = { - ...MOCK_QUOTES[TEST_AGG_ID], + ...getMockQuotes()[TEST_AGG_ID_1], aggregator: bestAggId, destinationAmount: ethers.BigNumber.from( - MOCK_QUOTES[TEST_AGG_ID].destinationAmount, + getMockQuotes()[TEST_AGG_ID_1].destinationAmount, ) - .add(1) + .add((100e18).toString()) .toString(), } - const quotes = { ...MOCK_QUOTES, [bestAggId]: bestQuote } + const quotes = { ...getMockQuotes(), [bestAggId]: bestQuote } fetchTradesInfoStub.resolves(quotes) // Make it so approval is not required @@ -405,7 +404,7 @@ describe('SwapsController', function () { }) it('does not mark as best quote if no conversion rate exists for destination token', async function () { - fetchTradesInfoStub.resolves(MOCK_QUOTES) + fetchTradesInfoStub.resolves(getMockQuotes()) // Make it so approval is not required sandbox @@ -762,4 +761,150 @@ describe('SwapsController', function () { }) }) }) + + describe('utils', function () { + describe('getMedian', function () { + const { getMedian } = utils + + it('calculates median correctly with uneven sample', function () { + const values = [3, 2, 6].map((value) => new BigNumber(value)) + const median = getMedian(values) + + assert.strictEqual( + median.toNumber(), 3, + 'should have returned correct median', + ) + }) + + it('calculates median correctly with even sample', function () { + const values = [3, 2, 2, 6].map((value) => new BigNumber(value)) + const median = getMedian(values) + + assert.strictEqual( + median.toNumber(), 2.5, + 'should have returned correct median', + ) + }) + + it('throws on empty or non-array sample', function () { + assert.throws( + () => getMedian([]), + 'should throw on empty array', + ) + + assert.throws( + () => getMedian(), + 'should throw on non-array param', + ) + + assert.throws( + () => getMedian({}), + 'should throw on non-array param', + ) + }) + }) + }) }) + +function getMockQuotes () { + return { + [TEST_AGG_ID_1]: { + 'trade': { + 'from': '0xe18035bf8712672935fdb4e5e431b1a0183d2dfc', + 'value': '0x0', + 'gas': '0x61a80', // 4e5 + 'to': '0x881D40237659C251811CEC9c364ef91dC08D300C', + }, + 'sourceAmount': '10000000000000000000', // 10e18 + 'destinationAmount': '20000000000000000000', // 20e18 + 'error': null, + 'sourceToken': '0x6b175474e89094c44da98b954eedeac495271d0f', + 'destinationToken': '0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48', + 'approvalNeeded': null, + 'maxGas': 600000, + 'averageGas': 120000, + 'estimatedRefund': 80000, + 'fetchTime': 607, + 'aggregator': TEST_AGG_ID_1, + 'aggType': 'AGG', + 'slippage': 2, + 'sourceTokenInfo': { + 'address': '0x6b175474e89094c44da98b954eedeac495271d0f', + 'symbol': 'DAI', + 'decimals': 18, + 'iconUrl': 'https://foo.bar/logo.png', + }, + 'destinationTokenInfo': { + 'address': '0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48', + 'symbol': 'USDC', + 'decimals': 18, + }, + }, + + [TEST_AGG_ID_BEST]: { + 'trade': { + 'from': '0xe18035bf8712672935fdb4e5e431b1a0183d2dfc', + 'value': '0x0', + 'gas': '0x61a80', + 'to': '0x881D40237659C251811CEC9c364ef91dC08D300C', + }, + 'sourceAmount': '10000000000000000000', + 'destinationAmount': '25000000000000000000', // 25e18 + 'error': null, + 'sourceToken': '0x6b175474e89094c44da98b954eedeac495271d0f', + 'destinationToken': '0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48', + 'approvalNeeded': null, + 'maxGas': 1100000, + 'averageGas': 411000, + 'estimatedRefund': 343090, + 'fetchTime': 1003, + 'aggregator': TEST_AGG_ID_BEST, + 'aggType': 'AGG', + 'slippage': 2, + 'sourceTokenInfo': { + 'address': '0x6b175474e89094c44da98b954eedeac495271d0f', + 'symbol': 'DAI', + 'decimals': 18, + 'iconUrl': 'https://foo.bar/logo.png', + }, + 'destinationTokenInfo': { + 'address': '0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48', + 'symbol': 'USDC', + 'decimals': 18, + }, + }, + + [TEST_AGG_ID_2]: { + 'trade': { + 'from': '0xe18035bf8712672935fdb4e5e431b1a0183d2dfc', + 'value': '0x0', + 'gas': '0x61a80', + 'to': '0x881D40237659C251811CEC9c364ef91dC08D300C', + }, + 'sourceAmount': '10000000000000000000', + 'destinationAmount': '22000000000000000000', // 22e18 + 'error': null, + 'sourceToken': '0x6b175474e89094c44da98b954eedeac495271d0f', + 'destinationToken': '0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48', + 'approvalNeeded': null, + 'maxGas': 368000, + 'averageGas': 197000, + 'estimatedRefund': 18205, + 'fetchTime': 1354, + 'aggregator': TEST_AGG_ID_2, + 'aggType': 'AGG', + 'slippage': 2, + 'sourceTokenInfo': { + 'address': '0x6b175474e89094c44da98b954eedeac495271d0f', + 'symbol': 'DAI', + 'decimals': 18, + 'iconUrl': 'https://foo.bar/logo.png', + }, + 'destinationTokenInfo': { + 'address': '0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48', + 'symbol': 'USDC', + 'decimals': 18, + }, + }, + } +}