Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: bump sor to 4.7.5 - fix: cached routes cache invalidation #747

Merged
merged 12 commits into from
Oct 28, 2024
4 changes: 2 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@uniswap/smart-order-router",
"version": "4.7.4",
"version": "4.7.5",
"description": "Uniswap Smart Order Router",
"main": "build/main/index.js",
"typings": "build/main/index.d.ts",
Expand Down
140 changes: 130 additions & 10 deletions src/routers/alpha-router/alpha-router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import {
TradeType,
} from '@uniswap/sdk-core';
import { TokenList } from '@uniswap/token-lists';
import { UniversalRouterVersion } from '@uniswap/universal-router-sdk';
import { Pool, Position, SqrtPriceMath, TickMath } from '@uniswap/v3-sdk';
import retry from 'async-retry';
import JSBI from 'jsbi';
Expand Down Expand Up @@ -149,6 +148,8 @@ import {
V4Route,
} from '../router';

import { UniversalRouterVersion } from '@uniswap/universal-router-sdk';
import { INTENT } from '../../util/intent';
import {
DEFAULT_ROUTING_CONFIG_BY_CHAIN,
ETH_GAS_STATION_API_URL,
Expand Down Expand Up @@ -324,16 +325,17 @@ export type AlphaRouterParams = {
*/
mixedSupported?: ChainId[];

/**
* The version of the universal router to use.
*/
universalRouterVersion?: UniversalRouterVersion;
jsy1218 marked this conversation as resolved.
Show resolved Hide resolved

/**
* The v4 pool params to be used for the v4 pool provider.
* fee tiers, tickspacings, and hook addresses
*/
v4PoolParams?: Array<[number, number, string]>;

/**
* We want to rollout the cached routes cache invalidation carefully.
* Because a wrong fix might impact prod success rate and/or latency.
*/
cachedRoutesCacheInvalidationFixRolloutPercentage?: number;
};

export class MapWithLowerCaseKey<V> extends Map<string, V> {
Expand Down Expand Up @@ -511,6 +513,10 @@ export type AlphaRouterConfig = {
* The version of the universal router to use.
*/
universalRouterVersion?: UniversalRouterVersion;
/**
* pass in routing-api intent to align the intent between routing-api and SOR
*/
intent?: INTENT;
};

export class AlphaRouter
Expand Down Expand Up @@ -550,8 +556,8 @@ export class AlphaRouter
protected v2Supported?: ChainId[];
protected v4Supported?: ChainId[];
protected mixedSupported?: ChainId[];
protected universalRouterVersion?: UniversalRouterVersion;
protected v4PoolParams?: Array<[number, number, string]>;
protected cachedRoutesCacheInvalidationFixRolloutPercentage?: number;

constructor({
chainId,
Expand Down Expand Up @@ -582,8 +588,8 @@ export class AlphaRouter
v2Supported,
v4Supported,
mixedSupported,
universalRouterVersion,
v4PoolParams,
cachedRoutesCacheInvalidationFixRolloutPercentage,
}: AlphaRouterParams) {
this.chainId = chainId;
this.provider = provider;
Expand Down Expand Up @@ -1047,8 +1053,9 @@ export class AlphaRouter
this.v2Supported = v2Supported ?? V2_SUPPORTED;
this.v4Supported = v4Supported ?? V4_SUPPORTED;
this.mixedSupported = mixedSupported ?? MIXED_SUPPORTED;
this.universalRouterVersion =
universalRouterVersion ?? UniversalRouterVersion.V1_2;

this.cachedRoutesCacheInvalidationFixRolloutPercentage =
cachedRoutesCacheInvalidationFixRolloutPercentage;
}

public async routeToRatio(
Expand Down Expand Up @@ -1645,6 +1652,102 @@ export class AlphaRouter
}
}

let newSetCachedRoutesPath = false;
const shouldEnableCachedRoutesCacheInvalidationFix =
Math.random() * 100 <
(this.cachedRoutesCacheInvalidationFixRolloutPercentage ?? 0);
jsy1218 marked this conversation as resolved.
Show resolved Hide resolved

// we have to write cached routes right before checking swapRouteRaw is null or not
// because getCachedRoutes in routing-api do not use the blocks-to-live to filter out the expired routes at all
// there's a possibility the cachedRoutes is always populated, but swapRouteFromCache is always null, because we don't update cachedRoutes in this case at all,
// as long as it's within 24 hours sliding window TTL
if (shouldEnableCachedRoutesCacheInvalidationFix) {
// theoretically, when routingConfig.intent === INTENT.CACHING, optimisticCachedRoutes should be false
// so that we can always pass in cachedRoutes?.notExpired(await blockNumber, !routingConfig.optimisticCachedRoutes)
// but just to be safe, we just hardcode true when checking the cached routes expiry for write update
// we decide to not check cached routes expiry in the read path anyway
if (!cachedRoutes?.notExpired(await blockNumber, true)) {
// optimisticCachedRoutes === false means at routing-api level, we only want to set cached routes during intent=caching, not intent=quote
// this means during the online quote endpoint path, we should not reset cached routes
if (routingConfig.intent === INTENT.CACHING) {
jsy1218 marked this conversation as resolved.
Show resolved Hide resolved
// due to fire and forget nature, we already take note that we should set new cached routes during the new path
newSetCachedRoutesPath = true;
metric.putMetric(`SetCachedRoute_NewPath`, 1, MetricLoggerUnit.Count);

// there's a chance that swapRouteFromChain might be populated already,
// when there's no cachedroutes in the dynamo DB.
// in that case, we don't try to swap route from chain again
const swapRouteFromChainAgain =
swapRouteFromChain ??
// we have to intentionally await here, because routing-api lambda has a chance to return the swapRoute/swapRouteWithSimulation
// before the routing-api quote handler can finish running getSwapRouteFromChain (getSwapRouteFromChain is runtime intensive)
(await this.getSwapRouteFromChain(
amount,
currencyIn,
currencyOut,
protocols,
quoteCurrency,
tradeType,
routingConfig,
v3GasModel,
v4GasModel,
mixedRouteGasModel,
gasPriceWei,
v2GasModel,
swapConfig,
providerConfig
));

if (swapRouteFromChainAgain) {
const routesToCache = CachedRoutes.fromRoutesWithValidQuotes(
jsy1218 marked this conversation as resolved.
Show resolved Hide resolved
swapRouteFromChainAgain.routes,
this.chainId,
currencyIn,
currencyOut,
protocols.sort(),
await blockNumber,
tradeType,
amount.toExact()
);

if (routesToCache) {
// Attempt to insert the entry in cache. This is fire and forget promise.
// The catch method will prevent any exception from blocking the normal code execution.
this.routeCachingProvider
?.setCachedRoute(routesToCache, amount)
.then((success) => {
const status = success ? 'success' : 'rejected';
metric.putMetric(
`SetCachedRoute_NewPath_${status}`,
1,
MetricLoggerUnit.Count
);
})
.catch((reason) => {
log.error(
{
reason: reason,
tokenPair: this.tokenPairSymbolTradeTypeChainId(
currencyIn,
currencyOut,
tradeType
),
},
`SetCachedRoute NewPath failure`
);

metric.putMetric(
`SetCachedRoute_NewPath_failure`,
1,
MetricLoggerUnit.Count
);
});
}
}
}
}
}

if (!swapRouteRaw) {
return null;
}
Expand All @@ -1659,12 +1762,29 @@ export class AlphaRouter
estimatedGasUsedGasToken,
} = swapRouteRaw;

// we intentionally dont add shouldEnableCachedRoutesCacheInvalidationFix in if condition below
// because we know cached routes in prod dont filter by blocks-to-live
// so that we know that swapRouteFromChain is always not populated, because
// if (!cachedRoutes || cacheMode !== CacheMode.Livemode) above always have the cachedRoutes as populated
if (
this.routeCachingProvider &&
routingConfig.writeToCachedRoutes &&
cacheMode !== CacheMode.Darkmode &&
swapRouteFromChain
) {
if (newSetCachedRoutesPath) {
// SetCachedRoute_NewPath and SetCachedRoute_OldPath metrics might have counts during short timeframe.
// over time, we should expect to see less SetCachedRoute_OldPath metrics count.
// in AWS metrics, one can investigate, by:
// 1) seeing the overall metrics count of SetCachedRoute_NewPath and SetCachedRoute_OldPath. SetCachedRoute_NewPath should steadily go up, while SetCachedRoute_OldPath should go down.
// 2) using the same requestId, one should see eventually when SetCachedRoute_NewPath metric is logged, SetCachedRoute_OldPath metric should not be called.
metric.putMetric(
`SetCachedRoute_OldPath_INTENT_${routingConfig.intent}`,
1,
MetricLoggerUnit.Count
);
}

// Generate the object to be cached
const routesToCache = CachedRoutes.fromRoutesWithValidQuotes(
swapRouteFromChain.routes,
Expand Down
1 change: 1 addition & 0 deletions src/util/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
export * from './addresses';
export * from './amounts';
export * from './chains';
export * from './intent';
export * from './log';
export * from './metric';
export * from './protocols';
Expand Down
8 changes: 8 additions & 0 deletions src/util/intent.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// NOTE: intent is a routing-api concept,
// but we have to introduce this strongly-typed enum in SOR to ensure some codepath only gets executed during async path
export enum INTENT {
CACHING = 'caching',
QUOTE = 'quote',
SWAP = 'swap',
PRICING = 'pricing',
}
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,8 @@ import {
WETH9,
WLD_WORLDCHAIN,
WNATIVE_ON,
WRAPPED_NATIVE_CURRENCY
WRAPPED_NATIVE_CURRENCY,
CacheMode
} from '../../../../src';
import { PortionProvider } from '../../../../src/providers/portion-provider';
import {
Expand All @@ -121,6 +122,12 @@ import {
} from '../../../test-util/mock-data';
import { WHALES } from '../../../test-util/whales';
import { V4SubgraphProvider } from '../../../../build/main';
import {
InMemoryRouteCachingProvider
} from '../../../unit/providers/caching/route/test-util/inmemory-route-caching-provider';
import {
getInvalidCachedRoutesStub
} from '../../../unit/routers/alpha-router/gas-models/test-util/mocked-dependencies';

const FORK_BLOCK = 20444945;
const UNIVERSAL_ROUTER_ADDRESS_V1_2 = UNIVERSAL_ROUTER_ADDRESS_BY_CHAIN(UniversalRouterVersion.V1_2, 1);
Expand Down Expand Up @@ -770,6 +777,7 @@ describe('alpha router integration', () => {
v3PoolProvider,
v4PoolProvider,
simulator: ethEstimateGasSimulator,
cachedRoutesCacheInvalidationFixRolloutPercentage: 100,
});

feeOnTransferAlphaRouter = new AlphaRouter({
Expand All @@ -779,6 +787,7 @@ describe('alpha router integration', () => {
v2PoolProvider: cachingV2PoolProvider,
v3PoolProvider,
simulator,
cachedRoutesCacheInvalidationFixRolloutPercentage: 100,
});
});

Expand Down Expand Up @@ -3113,6 +3122,7 @@ describe('alpha router integration', () => {
provider: hardhat.providers[0]!,
multicall2Provider,
gasPriceProvider,
cachedRoutesCacheInvalidationFixRolloutPercentage: 100,
});

const swap = await customAlphaRouter.route(
Expand All @@ -3133,6 +3143,71 @@ describe('alpha router integration', () => {

await validateSwapRoute(quote, quoteGasAdjusted, tradeType, 100, 10);
});

// This test is easy to set up at SOR level, but hard at routing-api level
// because at routing-api level, its difficult to trigger a quote to ensure an invalid cached routes get persisted into DB
// then we want to repro the invalid cached routes at SOR level
it(`erc20 -> erc20 cached routes cache invalidation`, async () => {
// invalid cached routes test setup is only set for exact-in in getInvalidCachedRoutesStub
if (tradeType !== TradeType.EXACT_INPUT) {
return
}

const tokenIn = USDC_MAINNET;
const tokenOut = DAI_MAINNET;
const amount = parseAmount('1.1', tokenIn);
const routeCachingProvider = new InMemoryRouteCachingProvider();
routeCachingProvider.cacheMode = CacheMode.Livemode;
routeCachingProvider.blocksToLive = Number.MAX_VALUE;
routeCachingProvider.expired = false;

// Insert a invalid cached routes for this test purpose to make sure it can get cleaned up
await routeCachingProvider.setCachedRoute(getInvalidCachedRoutesStub(FORK_BLOCK)!, CurrencyAmount.fromRawAmount(USDC_MAINNET, 100));

// Create a new AlphaRouter
const customAlphaRouter: AlphaRouter = new AlphaRouter({
chainId: 1,
provider: hardhat.providers[0]!,
multicall2Provider,
routeCachingProvider,
cachedRoutesCacheInvalidationFixRolloutPercentage: 100
});

routeCachingProvider.expired = true;

let swap = await customAlphaRouter.route(
amount,
getQuoteToken(tokenIn, tokenOut, tradeType),
tradeType,
undefined,
{
...ROUTING_CONFIG,
protocols: [Protocol.V2, Protocol.V3, Protocol.MIXED],
optimisticCachedRoutes: false // to trigger cache invalidation during intent=caching
},
);
// first time the swap is gonna be null, because even with the new set cached routes codepath,
// i intentionally made it a fire and forget, i.e. non-blocking on the aloha-router request processing path
// first time swap from cache from the request processing path will be null
expect(swap).toBeNull(); // first time expect swapRouteFromCache to be null

swap = await customAlphaRouter.route(
amount,
getQuoteToken(tokenIn, tokenOut, tradeType),
tradeType,
undefined,
{
...ROUTING_CONFIG,
optimisticCachedRoutes: false // to make sure the updated in-memory cache contains the valid cached routes now
},
);

// second time we will ensure the quote exists
// because the first time we had the sets cached routes in the new code path
// with second time we will have the cache hit from the new codepath cached routes
expect(swap).toBeDefined();
expect(swap).not.toBeNull();
});
});
}

Expand Down Expand Up @@ -3607,6 +3682,7 @@ describe('quote for other networks', () => {
[3000, 60, '0x0000000000000000000000000000000000000020'],
]
),
cachedRoutesCacheInvalidationFixRolloutPercentage: 100,
});
} else {
alphaRouter = new AlphaRouter({
Expand All @@ -3621,6 +3697,7 @@ describe('quote for other networks', () => {
[3000, 60, '0x0000000000000000000000000000000000000020'],
]
),
cachedRoutesCacheInvalidationFixRolloutPercentage: 100,
});
}
});
Expand Down
Loading
Loading