Skip to content

Commit

Permalink
fix: don't use cachedRoutes if intent.caching (1% experiment) (#833)
Browse files Browse the repository at this point in the history
* fix: don't use cachedRoutes if intent.caching (1% experiment)

* invert experiment flag
  • Loading branch information
xrsv authored Mar 3, 2025
1 parent dc61ab3 commit e3ba55c
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 7 deletions.
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.19.0",
"version": "4.19.1",
"description": "Uniswap Smart Order Router",
"main": "build/main/index.js",
"typings": "build/main/index.d.ts",
Expand Down
25 changes: 24 additions & 1 deletion src/routers/alpha-router/alpha-router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1470,7 +1470,11 @@ export class AlphaRouter
: 'N/A',
});

if (routingConfig.useCachedRoutes && cacheMode !== CacheMode.Darkmode) {
if (
routingConfig.useCachedRoutes &&
cacheMode !== CacheMode.Darkmode &&
AlphaRouter.isAllowedToEnterCachedRoutes(routingConfig.intent)
) {
if (enabledAndRequestedProtocolsMatch) {
if (
protocols.includes(Protocol.V4) &&
Expand Down Expand Up @@ -3195,4 +3199,23 @@ export class AlphaRouter
}
);
}

// Percentage of time we want to skip cached routes for the experiment.
// Starting with 1% to gradually roll out the change.
public static readonly CACHED_ROUTES_SKIP_EXPERIMENT_FLAG_PERCENTAGE = 0.01;

// We want to skip cached routes access whenever "intent === INTENT.CACHING".
// To verify this functionality though, we want to start by using a percentage of the time.
public static isAllowedToEnterCachedRoutes(intent?: INTENT): boolean {
// Check if we should run the experiment
const shouldRunExperiment =
Math.random() < AlphaRouter.CACHED_ROUTES_SKIP_EXPERIMENT_FLAG_PERCENTAGE;
if (shouldRunExperiment) {
// For the experiment group, we want to skip the cached routes access if the intent is caching
return intent !== INTENT.CACHING;
}

// For the control group, we always allow access to cached routes.
return true;
}
}
35 changes: 32 additions & 3 deletions test/unit/routers/alpha-router/alpha-router.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ import {
import {
InMemoryRouteCachingProvider
} from '../../providers/caching/route/test-util/inmemory-route-caching-provider';
import { INTENT } from '../../../../src/util/intent';

const helper = require('../../../../src/routers/alpha-router/functions/calculate-ratio-amount-in');

Expand Down Expand Up @@ -1616,9 +1617,9 @@ describe('alpha router', () => {
});

test('with V1.2 - skips cache when V2 is missing from requested protocols', async () => {
const v1_2Params = {
...swapParams,
version: UniversalRouterVersion.V1_2
const v1_2Params = {
...swapParams,
version: UniversalRouterVersion.V1_2
};

const swap = await alphaRouter.route(
Expand Down Expand Up @@ -3087,6 +3088,34 @@ describe('alpha router', () => {
});
});
});

describe('isAllowedToEnterCachedRoutes', () => {
let randomStub: sinon.SinonStub;

beforeEach(() => {
randomStub = sinon.stub(Math, 'random');
});

afterEach(() => {
randomStub.restore();
});

test('returns correct values based on random percentage and intent', () => {
// Test when random is above experiment threshold (control group - should always return true)
randomStub.returns(AlphaRouter.CACHED_ROUTES_SKIP_EXPERIMENT_FLAG_PERCENTAGE + 0.01);
expect(AlphaRouter.isAllowedToEnterCachedRoutes(INTENT.CACHING)).toBe(true);
expect(AlphaRouter.isAllowedToEnterCachedRoutes(INTENT.QUOTE)).toBe(true);
expect(AlphaRouter.isAllowedToEnterCachedRoutes(undefined)).toBe(true);

// Test when random is below experiment threshold (experiment group)
randomStub.returns(AlphaRouter.CACHED_ROUTES_SKIP_EXPERIMENT_FLAG_PERCENTAGE - 0.001);
// Should return false only for CACHING intent
expect(AlphaRouter.isAllowedToEnterCachedRoutes(INTENT.CACHING)).toBe(false);
// Should return true for other intents
expect(AlphaRouter.isAllowedToEnterCachedRoutes(INTENT.QUOTE)).toBe(true);
expect(AlphaRouter.isAllowedToEnterCachedRoutes(undefined)).toBe(true);
});
});
});

type GetQuotesManyExactInFnParams = {
Expand Down

0 comments on commit e3ba55c

Please sign in to comment.