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

package.json

@@ -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",

src/routers/alpha-router/alpha-router.ts

@@ -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';
@@ -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,
@@ -324,16 +325,17 @@ export type AlphaRouterParams = {
    */
   mixedSupported?: ChainId[];
 
-  /**
-   * The version of the universal router to use.
-   */
-  universalRouterVersion?: UniversalRouterVersion;
-
   /**
    * 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> {
@@ -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
@@ -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,
@@ -582,8 +588,8 @@ export class AlphaRouter
     v2Supported,
     v4Supported,
     mixedSupported,
-    universalRouterVersion,
     v4PoolParams,
+    cachedRoutesCacheInvalidationFixRolloutPercentage,
   }: AlphaRouterParams) {
     this.chainId = chainId;
     this.provider = provider;
@@ -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(
@@ -1645,6 +1652,77 @@ export class AlphaRouter
       }
     }
 
+    let newSetCachedRoutesPath = false;
+    const shouldEnableCachedRoutesCacheInvalidationFix =
+      Math.random() * 100 <
+      (this.cachedRoutesCacheInvalidationFixRolloutPercentage ?? 0);
+
+    // 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) {
+          // 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(
+              swapRouteFromChainAgain.routes,
+              this.chainId,
+              currencyIn,
+              currencyOut,
+              protocols.sort(),
+              await blockNumber,
+              tradeType,
+              amount.toExact()
+            );
+
+            this.setCachedRoutesAndLog(
+              amount,
+              currencyIn,
+              currencyOut,
+              tradeType,
+              'SetCachedRoute_NewPath',
+              routesToCache
+            );
+          }
+        }
+      }
+    }
+
     if (!swapRouteRaw) {
       return null;
     }
@@ -1659,12 +1737,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,
@@ -1677,45 +1772,14 @@ export class AlphaRouter
         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_${status}`,
-              1,
-              MetricLoggerUnit.Count
-            );
-          })
-          .catch((reason) => {
-            log.error(
-              {
-                reason: reason,
-                tokenPair: this.tokenPairSymbolTradeTypeChainId(
-                  currencyIn,
-                  currencyOut,
-                  tradeType
-                ),
-              },
-              `SetCachedRoute failure`
-            );
-
-            metric.putMetric(
-              `SetCachedRoute_failure`,
-              1,
-              MetricLoggerUnit.Count
-            );
-          });
-      } else {
-        metric.putMetric(
-          `SetCachedRoute_unnecessary`,
-          1,
-          MetricLoggerUnit.Count
-        );
-      }
+      this.setCachedRoutesAndLog(
+        amount,
+        currencyIn,
+        currencyOut,
+        tradeType,
+        'SetCachedRoute_OldPath',
+        routesToCache
+      );
     }
 
     metric.putMetric(
@@ -1838,6 +1902,55 @@ export class AlphaRouter
     return swapRoute;
   }
 
+  private async setCachedRoutesAndLog(
+    amount: CurrencyAmount,
+    currencyIn: Currency,
+    currencyOut: Currency,
+    tradeType: TradeType,
+    metricsPrefix: string,
+    routesToCache?: CachedRoutes
+  ): Promise<void> {
+    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(
+            `${metricsPrefix}_${status}`,
+            1,
+            MetricLoggerUnit.Count
+          );
+        })
+        .catch((reason) => {
+          log.error(
+            {
+              reason: reason,
+              tokenPair: this.tokenPairSymbolTradeTypeChainId(
+                currencyIn,
+                currencyOut,
+                tradeType
+              ),
+            },
+            `SetCachedRoute failure`
+          );
+
+          metric.putMetric(
+            `${metricsPrefix}_failure`,
+            1,
+            MetricLoggerUnit.Count
+          );
+        });
+    } else {
+      metric.putMetric(
+        `${metricsPrefix}_unnecessary`,
+        1,
+        MetricLoggerUnit.Count
+      );
+    }
+  }
+
   private async getSwapRouteFromCache(
     currencyIn: Currency,
     currencyOut: Currency,

src/util/index.ts

@@ -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';

src/util/intent.ts

@@ -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',
+}