From 633ee349ee249a1aad03f4382ee0bf8914c4233c Mon Sep 17 00:00:00 2001 From: "Siyu Jiang (See-You John)" <91580504+jsy1218@users.noreply.github.com> Date: Thu, 5 Sep 2024 19:32:09 -0700 Subject: [PATCH] feat: exclude certain protocol versions from mixed routing (#686) * feat: exclude certain protocol versions from mixed routing * feat: exclude certain protocol versions from mixed routing * fix: base gas deviation is 53% * only wipe out cached routes when intent=caching, meaning it's offline request * fix unit test --- src/routers/alpha-router/alpha-router.ts | 11 +- .../alpha-router/functions/best-swap-route.ts | 6 +- .../alpha-router/quoters/mixed-quoter.ts | 8 +- src/util/methodParameters.ts | 12 +- src/util/routes.ts | 70 ++++++++++- .../alpha-router.integration.test.ts | 2 +- test/unit/providers/token-fee-fetcher.test.ts | 6 +- .../alpha-router/functions/routes.test.ts | 112 ++++++++++++++++++ 8 files changed, 212 insertions(+), 15 deletions(-) create mode 100644 test/unit/routers/alpha-router/functions/routes.test.ts diff --git a/src/routers/alpha-router/alpha-router.ts b/src/routers/alpha-router/alpha-router.ts index d4b79bd0b..b854aaf60 100644 --- a/src/routers/alpha-router/alpha-router.ts +++ b/src/routers/alpha-router/alpha-router.ts @@ -87,9 +87,10 @@ import { import { IV3SubgraphProvider } from '../../providers/v3/subgraph-provider'; import { Erc20__factory } from '../../types/other/factories/Erc20__factory'; import { + shouldWipeoutCachedRoutes, SWAP_ROUTER_02_ADDRESSES, V4_SUPPORTED, - WRAPPED_NATIVE_CURRENCY + WRAPPED_NATIVE_CURRENCY, } from '../../util'; import { CurrencyAmount } from '../../util/amounts'; import { @@ -399,6 +400,10 @@ export type AlphaRouterConfig = { * will be used. */ protocols?: Protocol[]; + /** + * The protocols-version pools to be excluded from the mixed routes. + */ + excludedProtocolsFromMixed?: Protocol[]; /** * Config for selecting which pools to consider routing via on V2. */ @@ -1356,6 +1361,10 @@ export class AlphaRouter ); } + if (shouldWipeoutCachedRoutes(cachedRoutes, routingConfig)) { + cachedRoutes = undefined; + } + metric.putMetric( routingConfig.useCachedRoutes ? 'GetQuoteUsingCachedRoutes' diff --git a/src/routers/alpha-router/functions/best-swap-route.ts b/src/routers/alpha-router/functions/best-swap-route.ts index 641811ff0..a09407b50 100644 --- a/src/routers/alpha-router/functions/best-swap-route.ts +++ b/src/routers/alpha-router/functions/best-swap-route.ts @@ -500,7 +500,11 @@ export async function getBestSwapRouteBy( // If swapping on an L2 that includes a L1 security fee, calculate the fee and include it in the gas adjusted quotes if (HAS_L1_FEE.includes(chainId)) { // ROUTE-249: consoliate L1 + L2 gas fee adjustment within best-swap-route - if (v2GasModel == undefined && v3GasModel == undefined && v4GasModel == undefined) { + if ( + v2GasModel == undefined && + v3GasModel == undefined && + v4GasModel == undefined + ) { throw new Error("Can't compute L1 gas fees."); } else { // Before v2 deploy everywhere, a quote on L2 can only go through v3 protocol, diff --git a/src/routers/alpha-router/quoters/mixed-quoter.ts b/src/routers/alpha-router/quoters/mixed-quoter.ts index 8ef8445f8..05b1644fe 100644 --- a/src/routers/alpha-router/quoters/mixed-quoter.ts +++ b/src/routers/alpha-router/quoters/mixed-quoter.ts @@ -15,6 +15,7 @@ import { } from '../../../providers'; import { CurrencyAmount, + excludeProtocolPoolRouteFromMixedRoute, log, metric, MetricLoggerUnit, @@ -154,6 +155,11 @@ export class MixedQuoter extends BaseQuoter< maxSwapsPerPath ); + const protocolExcludedRoutes = excludeProtocolPoolRouteFromMixedRoute( + routes, + routingConfig.excludedProtocolsFromMixed + ); + metric.putMetric( 'MixedGetRoutesLoad', Date.now() - beforeGetRoutes, @@ -161,7 +167,7 @@ export class MixedQuoter extends BaseQuoter< ); return { - routes, + routes: protocolExcludedRoutes, candidatePools, }; } diff --git a/src/util/methodParameters.ts b/src/util/methodParameters.ts index 1d5683f87..90a6cc192 100644 --- a/src/util/methodParameters.ts +++ b/src/util/methodParameters.ts @@ -4,14 +4,10 @@ import { SwapRouter as SwapRouter02, Trade, } from '@uniswap/router-sdk'; +import { ChainId, Currency, TradeType } from '@uniswap/sdk-core'; import { - ChainId, - Currency, - TradeType, -} from '@uniswap/sdk-core'; -import { - UNIVERSAL_ROUTER_ADDRESS, SwapRouter as UniversalRouter, + UNIVERSAL_ROUTER_ADDRESS, } from '@uniswap/universal-router-sdk'; import { Route as V2RouteRaw } from '@uniswap/v2-sdk'; import { Route as V3RouteRaw } from '@uniswap/v3-sdk'; @@ -23,12 +19,12 @@ import { MethodParameters, MixedRouteWithValidQuote, RouteWithValidQuote, - SWAP_ROUTER_02_ADDRESSES, SwapOptions, SwapType, + SWAP_ROUTER_02_ADDRESSES, V2RouteWithValidQuote, V3RouteWithValidQuote, - V4RouteWithValidQuote + V4RouteWithValidQuote, } from '..'; export function buildTrade( diff --git a/src/util/routes.ts b/src/util/routes.ts index 05ca43e8e..61d5dd185 100644 --- a/src/util/routes.ts +++ b/src/util/routes.ts @@ -5,12 +5,16 @@ import { Pool as V3Pool } from '@uniswap/v3-sdk'; import { Pool as V4Pool } from '@uniswap/v4-sdk'; import _ from 'lodash'; -import { RouteWithValidQuote } from '../routers/alpha-router'; -import { SupportedRoutes } from '../routers/router'; +import { + AlphaRouterConfig, + RouteWithValidQuote, +} from '../routers/alpha-router'; +import { MixedRoute, SupportedRoutes } from '../routers/router'; import { V3_CORE_FACTORY_ADDRESSES } from './addresses'; import { CurrencyAmount } from '.'; +import { CachedRoutes } from '../providers'; export const routeToTokens = (route: SupportedRoutes): Currency[] => { switch (route.protocol) { @@ -134,3 +138,65 @@ export const routeAmountsToString = ( return _.join(routeStrings, ', '); }; + +export function shouldWipeoutCachedRoutes( + cachedRoutes?: CachedRoutes, + routingConfig?: AlphaRouterConfig +): boolean { + // In case of optimisticCachedRoutes, we don't want to wipe out the cache + // This is because the upstream client will indicate that it's a perf sensitive (likely online) request, + // such that we should still use the cached routes. + // In case of routing-api, + // when intent=quote, optimisticCachedRoutes will be true, it means it's an online quote request, and we should use the cached routes. + // when intent=caching, optimisticCachedRoutes will be false, it means it's an async routing lambda invocation for the benefit of + // non-perf-sensitive, so that we can nullify the retrieved cached routes, if certain condition meets. + if (routingConfig?.optimisticCachedRoutes) { + return false; + } + + const containsExcludedProtocolPools = cachedRoutes?.routes.find((route) => { + switch (route.protocol) { + case Protocol.MIXED: + return ( + (route.route as MixedRoute).pools.filter((pool) => { + return poolIsInExcludedProtocols( + pool, + routingConfig?.excludedProtocolsFromMixed + ); + }).length > 0 + ); + default: + return false; + } + }); + + return containsExcludedProtocolPools !== undefined; +} + +export function excludeProtocolPoolRouteFromMixedRoute( + mixedRoutes: MixedRoute[], + excludedProtocolsFromMixed?: Protocol[] +): MixedRoute[] { + return mixedRoutes.filter((route) => { + return ( + route.pools.filter((pool) => { + return poolIsInExcludedProtocols(pool, excludedProtocolsFromMixed); + }).length == 0 + ); + }); +} + +function poolIsInExcludedProtocols( + pool: V4Pool | V3Pool | Pair, + excludedProtocolsFromMixed?: Protocol[] +): boolean { + if (pool instanceof V4Pool) { + return excludedProtocolsFromMixed?.includes(Protocol.V4) ?? false; + } else if (pool instanceof V3Pool) { + return excludedProtocolsFromMixed?.includes(Protocol.V3) ?? false; + } else if (pool instanceof Pair) { + return excludedProtocolsFromMixed?.includes(Protocol.V2) ?? false; + } else { + return false; + } +} diff --git a/test/integ/routers/alpha-router/alpha-router.integration.test.ts b/test/integ/routers/alpha-router/alpha-router.integration.test.ts index 521953838..1f454385f 100644 --- a/test/integ/routers/alpha-router/alpha-router.integration.test.ts +++ b/test/integ/routers/alpha-router/alpha-router.integration.test.ts @@ -143,7 +143,7 @@ const GAS_ESTIMATE_DEVIATION_PERCENT: { [chainId in ChainId]: number } = { [ChainId.MOONBEAM]: 30, [ChainId.BNB]: 82, [ChainId.AVALANCHE]: 45, - [ChainId.BASE]: 50, + [ChainId.BASE]: 53, [ChainId.BASE_GOERLI]: 30, [ChainId.ZORA]: 50, [ChainId.ZORA_SEPOLIA]: 30, diff --git a/test/unit/providers/token-fee-fetcher.test.ts b/test/unit/providers/token-fee-fetcher.test.ts index c5a3fd0f8..7a705768c 100644 --- a/test/unit/providers/token-fee-fetcher.test.ts +++ b/test/unit/providers/token-fee-fetcher.test.ts @@ -6,6 +6,7 @@ import { } from '../../../src/providers/token-fee-fetcher'; import { BITBOY, BOYS, BULLET, DFNDR } from '../../test-util/mock-data'; import dotenv from 'dotenv'; +import { ProviderConfig } from '../../../src/providers/provider'; const each = require("jest-each").default; dotenv.config(); @@ -16,11 +17,14 @@ describe('TokenFeeFetcher', () => { [ChainId.MAINNET, WETH9[ChainId.MAINNET]!, DFNDR, true, false], [ChainId.BASE, WETH9[ChainId.BASE]!, BOYS, false, false], ]).it('Fetch non-FOT and FOT, should only return FOT', async (chain: ChainId, inputToken: Token, outputToken: Token, feeTakenOnTransfer?: boolean, externalTransferFailed?: boolean) => { + const providerConfig: ProviderConfig = { + blockNumber: chain === ChainId.MAINNET ? 20686752 : 19395859, + } const chainProvider = ID_TO_PROVIDER(chain); const provider = new JsonRpcProvider(chainProvider, chain); const tokenFeeFetcher = new OnChainTokenFeeFetcher(chain, provider); - const tokenFeeMap = await tokenFeeFetcher.fetchFees([inputToken.address, outputToken.address]) + const tokenFeeMap = await tokenFeeFetcher.fetchFees([inputToken.address, outputToken.address], providerConfig) expect(tokenFeeMap).not.toContain(inputToken.address) expect(tokenFeeMap[outputToken.address]).toBeDefined() diff --git a/test/unit/routers/alpha-router/functions/routes.test.ts b/test/unit/routers/alpha-router/functions/routes.test.ts new file mode 100644 index 000000000..5e64b1fab --- /dev/null +++ b/test/unit/routers/alpha-router/functions/routes.test.ts @@ -0,0 +1,112 @@ +import { + AlphaRouterConfig, + CachedRoute, + CachedRoutes, + DAI_MAINNET, + excludeProtocolPoolRouteFromMixedRoute, + MixedRoute, + shouldWipeoutCachedRoutes, + USDC_MAINNET +} from '../../../../../src'; +import { Protocol } from '@uniswap/router-sdk'; +import { ChainId, TradeType } from '@uniswap/sdk-core'; +import { + USDC_DAI, + USDC_DAI_LOW, + USDC_DAI_MEDIUM, + USDC_DAI_V4_LOW +} from '../../../../test-util/mock-data'; +import { + DEFAULT_ROUTING_CONFIG_BY_CHAIN +} from '../../../../../src/routers/alpha-router/config'; + +describe('routes', () => { + const mixedRoutes1 = new MixedRoute([USDC_DAI, USDC_DAI_LOW], USDC_MAINNET, DAI_MAINNET); + const cachedRoute1 = new CachedRoute({ + route: mixedRoutes1, + percent: 50, + }); + const mixedRoutes2 = new MixedRoute([USDC_DAI_V4_LOW, USDC_DAI_LOW], USDC_MAINNET, DAI_MAINNET); + const cachedRoute2 = new CachedRoute({ + route: mixedRoutes2, + percent: 50, + }); + const mixedRoutes3 = new MixedRoute([USDC_DAI, USDC_DAI_LOW, USDC_DAI_MEDIUM], USDC_MAINNET, DAI_MAINNET); + const cachedRoute3 = new CachedRoute({ + route: mixedRoutes3, + percent: 50, + }); + + const cachedRoutesIncludeRouteWithV4Pool = new CachedRoutes({ + routes: [cachedRoute1, cachedRoute2], + chainId: 1, + tokenIn: USDC_MAINNET, + tokenOut: DAI_MAINNET, + protocolsCovered: [Protocol.V2, Protocol.V3, Protocol.V4, Protocol.MIXED], + blockNumber: 1, + tradeType: TradeType.EXACT_INPUT, + originalAmount: '100', + blocksToLive: 100 + }); + + const cachedRoutesIncludeRouteWithoutV4Pool = new CachedRoutes({ + routes: [cachedRoute1, cachedRoute3], + chainId: 1, + tokenIn: USDC_MAINNET, + tokenOut: DAI_MAINNET, + protocolsCovered: [Protocol.V2, Protocol.V3, Protocol.V4, Protocol.MIXED], + blockNumber: 1, + tradeType: TradeType.EXACT_INPUT, + originalAmount: '100', + blocksToLive: 100 + }); + + test(`do not exclude any cached route for empty excluded protocols list`, async () => { + const routingConfig: AlphaRouterConfig = { + // @ts-ignore[TS7053] - complaining about switch being non exhaustive + ...DEFAULT_ROUTING_CONFIG_BY_CHAIN[ChainId.MAINNET], + protocols: [Protocol.V2, Protocol.V3, Protocol.V4, Protocol.MIXED], + excludedProtocolsFromMixed: [], + optimisticCachedRoutes: false, + }; + + expect(shouldWipeoutCachedRoutes(cachedRoutesIncludeRouteWithV4Pool, routingConfig)).toBeFalsy(); + }); + + test(`exclude cached route for V4 protocol`, async () => { + const routingConfig: AlphaRouterConfig = { + // @ts-ignore[TS7053] - complaining about switch being non exhaustive + ...DEFAULT_ROUTING_CONFIG_BY_CHAIN[ChainId.MAINNET], + protocols: [Protocol.V2, Protocol.V3, Protocol.V4, Protocol.MIXED], + excludedProtocolsFromMixed: [Protocol.V4], + optimisticCachedRoutes: false, + }; + expect(shouldWipeoutCachedRoutes(cachedRoutesIncludeRouteWithV4Pool, routingConfig)).toBeTruthy(); + }); + + test(`do not exclude cached route for V4 protocol`, async () => { + const routingConfig: AlphaRouterConfig = { + // @ts-ignore[TS7053] - complaining about switch being non exhaustive + ...DEFAULT_ROUTING_CONFIG_BY_CHAIN[ChainId.MAINNET], + protocols: [Protocol.V2, Protocol.V3, Protocol.V4, Protocol.MIXED], + excludedProtocolsFromMixed: [Protocol.V4], + optimisticCachedRoutes: false, + }; + expect(shouldWipeoutCachedRoutes(cachedRoutesIncludeRouteWithoutV4Pool, routingConfig)).toBeFalsy(); + }); + + test(`do not exclude any mixed route for empty excluded protocols list`, async () => { + const newMixedRoutes = excludeProtocolPoolRouteFromMixedRoute([mixedRoutes1, mixedRoutes2, mixedRoutes3], []); + expect(newMixedRoutes).toStrictEqual([mixedRoutes1, mixedRoutes2, mixedRoutes3]); + }); + + test(`exclude mixed route for V4 protocol`, async () => { + const newMixedRoutes = excludeProtocolPoolRouteFromMixedRoute([mixedRoutes1, mixedRoutes2, mixedRoutes3], [Protocol.V4]); + expect(newMixedRoutes).toStrictEqual([mixedRoutes1, mixedRoutes3]); + }); + + test(`do not exclude mixed route for V4 protocol`, async () => { + const newMixedRoutes = excludeProtocolPoolRouteFromMixedRoute([mixedRoutes1, mixedRoutes3], [Protocol.V4]); + expect(newMixedRoutes).toStrictEqual([mixedRoutes1, mixedRoutes3]); + }); +});