From 1b0ea82ac7866a71b74e3f16ee4d1eaf1cf3ccd1 Mon Sep 17 00:00:00 2001 From: Alexander Kropiunig Date: Sat, 16 May 2026 21:21:58 +0200 Subject: [PATCH] fix(web): preserve correct tickSpacing when migrating legacy feeTier URL parameter The URL migration registered as V1 in apps/web/src/features/Liquidity/parsers/migrations.ts converts the deprecated `feeTier` query parameter into the new `fee` FeeData object. For any feeTier value that is not one of the standard V3 fee amounts present in `TICK_SPACINGS` (1, 200, 300, 400, 500, 3000, 10000), the previous logic silently fell back to `TICK_SPACINGS[FeeAmount.MEDIUM]` (60): const tickSpacing = TICK_SPACINGS[feeTierNumber as FeeAmount] || TICK_SPACINGS[FeeAmount.MEDIUM] This caused legacy links pointing at custom V4 fee tiers (e.g. `feeTier=1000`, common for dynamic-fee hooks on Base) to land on the create-position page with a tickSpacing of 60 instead of the actual pool tickSpacing (20 for fee=1000), fracturing liquidity into a parallel non-existent pool. The issue is reported in Uniswap/interface#7939 and reproduces in the deprecated URL format used by Uniswap/interface#7940. The fix uses the same formula already adopted elsewhere in the codebase (`calculateTickSpacingFromFeeAmount` in features/Liquidity/utils/feeTiers.ts): const tickSpacing = TICK_SPACINGS[feeTierNumber as FeeAmount] ?? calculateTickSpacingFromFeeAmount(feeTierNumber) For every standard fee tier the lookup short-circuits, so behavior is unchanged. For custom fee tiers the SDK-canonical mathematical default is returned instead of MEDIUM, matching what `FeeTierSearchModal` and `usePoolData` use when constructing the same FeeData object from a custom fee amount. Adds apps/web/src/features/Liquidity/parsers/migrations.test.ts covering: - All 7 standard V3 fee tiers (regression guard) - Custom V4 fee tiers (1000, 1500, 2000, 2500, 4000, 7500, 250, 333, 50, 1) - The specific regression: feeTier=1000 must not coerce to tickSpacing=60 - isDynamic propagation, missing isDynamic, empty feeTier, currency migrations - Combined fee + currency migrations with deduplicated clearParams Fixes #7939 --- .../Liquidity/parsers/migrations.test.ts | 154 ++++++++++++++++++ .../features/Liquidity/parsers/migrations.ts | 11 +- 2 files changed, 163 insertions(+), 2 deletions(-) create mode 100644 apps/web/src/features/Liquidity/parsers/migrations.test.ts diff --git a/apps/web/src/features/Liquidity/parsers/migrations.test.ts b/apps/web/src/features/Liquidity/parsers/migrations.test.ts new file mode 100644 index 00000000000..c93f4e5aaec --- /dev/null +++ b/apps/web/src/features/Liquidity/parsers/migrations.test.ts @@ -0,0 +1,154 @@ +import { FeeAmount, TICK_SPACINGS } from '@uniswap/v3-sdk' +import { describe, expect, it } from 'vitest' +import { applyUrlMigrations } from '~/features/Liquidity/parsers/migrations' + +describe('applyUrlMigrations', () => { + describe('migrateFee', () => { + describe('standard V3 fee tiers (preserve existing behavior)', () => { + it.each([ + [FeeAmount.LOWEST, TICK_SPACINGS[FeeAmount.LOWEST]], + [FeeAmount.LOW_200, TICK_SPACINGS[FeeAmount.LOW_200]], + [FeeAmount.LOW_300, TICK_SPACINGS[FeeAmount.LOW_300]], + [FeeAmount.LOW_400, TICK_SPACINGS[FeeAmount.LOW_400]], + [FeeAmount.LOW, TICK_SPACINGS[FeeAmount.LOW]], + [FeeAmount.MEDIUM, TICK_SPACINGS[FeeAmount.MEDIUM]], + [FeeAmount.HIGH, TICK_SPACINGS[FeeAmount.HIGH]], + ])('maps standard feeTier=%i to canonical tickSpacing=%i', (feeAmount, expectedTickSpacing) => { + const result = applyUrlMigrations({ feeTier: String(feeAmount), isDynamic: false }) + + expect(result).not.toBeNull() + expect(result?.updatedParams.fee).toEqual({ + feeAmount, + tickSpacing: expectedTickSpacing, + isDynamic: false, + }) + expect(result?.clearParams).toEqual(expect.arrayContaining(['feeTier', 'isDynamic'])) + }) + + it('propagates isDynamic=true for standard fee tiers', () => { + const result = applyUrlMigrations({ feeTier: String(FeeAmount.LOW), isDynamic: true }) + + expect(result?.updatedParams.fee).toEqual({ + feeAmount: FeeAmount.LOW, + tickSpacing: TICK_SPACINGS[FeeAmount.LOW], + isDynamic: true, + }) + }) + }) + + describe('non-standard V4 fee tiers (regression: previously coerced to tickSpacing=60)', () => { + // tickSpacing = max(round(2 * feeAmount / 100), 1) + it.each([ + // feeAmount, expectedTickSpacing + [1000, 20], // 0.10% — exact reproduction case from issue + [1500, 30], + [2000, 40], + [2500, 50], + [4000, 80], + [7500, 150], + [250, 5], + [333, 7], + [50, 1], // very small, but >= 1 + [1, 1], // floor at 1 + ])( + 'derives tickSpacing=%2$i from non-standard feeTier=%1$i instead of falling back to 60', + (feeAmount, expectedTickSpacing) => { + const result = applyUrlMigrations({ feeTier: String(feeAmount), isDynamic: false }) + + expect(result?.updatedParams.fee?.tickSpacing).toBe(expectedTickSpacing) + expect(result?.updatedParams.fee?.feeAmount).toBe(feeAmount) + }, + ) + + it('does not coerce custom V4 fee tier 1000 to MEDIUM tick spacing of 60', () => { + const result = applyUrlMigrations({ feeTier: '1000', isDynamic: false }) + + // Regression guard: previous implementation fell through to TICK_SPACINGS[FeeAmount.MEDIUM] = 60 + // for any feeTier not present in TICK_SPACINGS. + expect(result?.updatedParams.fee?.tickSpacing).not.toBe(60) + expect(result?.updatedParams.fee?.tickSpacing).toBe(20) + }) + + it('preserves isDynamic flag for custom fee tiers', () => { + const result = applyUrlMigrations({ feeTier: '1000', isDynamic: true }) + + expect(result?.updatedParams.fee).toEqual({ + feeAmount: 1000, + tickSpacing: 20, + isDynamic: true, + }) + }) + + it('handles isDynamic=undefined as false (legacy URLs without the flag)', () => { + const result = applyUrlMigrations({ feeTier: '1000' }) + + expect(result?.updatedParams.fee?.isDynamic).toBe(false) + }) + }) + + it('returns null when no feeTier present and no other migrations apply', () => { + const result = applyUrlMigrations({}) + + expect(result).toBeNull() + }) + + it('does not migrate when feeTier is empty string', () => { + const result = applyUrlMigrations({ feeTier: '' }) + + expect(result).toBeNull() + }) + }) + + describe('migrateCurrency', () => { + it('migrates lowercase currencya to currencyA when currencyA is empty', () => { + const result = applyUrlMigrations({ + currencya: '0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48', + }) + + expect(result?.updatedParams.currencyA).toBe('0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48') + expect(result?.clearParams).toContain('currencya') + }) + + it('migrates lowercase currencyb to currencyB when currencyB is empty', () => { + const result = applyUrlMigrations({ + currencyb: '0xc02aaa39b223fe8d0a0e5c4f27ead9083c756cc2', + }) + + expect(result?.updatedParams.currencyB).toBe('0xc02aaa39b223fe8d0a0e5c4f27ead9083c756cc2') + expect(result?.clearParams).toContain('currencyb') + }) + + it('does not overwrite currencyA when both currencyA and currencya are present', () => { + const result = applyUrlMigrations({ + currencyA: '0xNew', + currencya: '0xOld', + }) + + expect(result?.updatedParams.currencyA).toBeUndefined() + }) + }) + + describe('combined migrations', () => { + it('applies both fee and currency migrations together and collects clearParams without duplicates', () => { + const result = applyUrlMigrations({ + feeTier: '1000', + isDynamic: true, + currencya: '0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48', + currencyb: '0xc02aaa39b223fe8d0a0e5c4f27ead9083c756cc2', + }) + + expect(result?.updatedParams.fee).toEqual({ + feeAmount: 1000, + tickSpacing: 20, + isDynamic: true, + }) + expect(result?.updatedParams.currencyA).toBe('0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48') + expect(result?.updatedParams.currencyB).toBe('0xc02aaa39b223fe8d0a0e5c4f27ead9083c756cc2') + + // clearParams should be deduplicated + const uniqueClearParams = new Set(result?.clearParams) + expect(uniqueClearParams.size).toBe(result?.clearParams.length) + expect(uniqueClearParams).toEqual(new Set(['feeTier', 'isDynamic', 'currencya', 'currencyb'])) + }) + }) +}) diff --git a/apps/web/src/features/Liquidity/parsers/migrations.ts b/apps/web/src/features/Liquidity/parsers/migrations.ts index 6cefb77c757..823aa1c503f 100644 --- a/apps/web/src/features/Liquidity/parsers/migrations.ts +++ b/apps/web/src/features/Liquidity/parsers/migrations.ts @@ -1,6 +1,6 @@ import { FeeAmount, TICK_SPACINGS } from '@uniswap/v3-sdk' import type { FeeData } from 'uniswap/src/features/positions/types' -import { isDynamicFeeTier } from '~/features/Liquidity/utils/feeTiers' +import { calculateTickSpacingFromFeeAmount, isDynamicFeeTier } from '~/features/Liquidity/utils/feeTiers' interface ParsedParams { // Current params @@ -40,7 +40,14 @@ function migrateFee(params: ParsedParams): UrlMigrationResult | null { // Migrate feeTier + isDynamic to fee object if (params.feeTier) { const feeTierNumber = Number(params.feeTier) - const tickSpacing = TICK_SPACINGS[feeTierNumber as FeeAmount] || TICK_SPACINGS[FeeAmount.MEDIUM] + // For standard V3 fee tiers, use the canonical tick spacing from the v3-sdk. + // For non-standard (e.g. custom V4) fee tiers, fall back to the formula + // tickSpacing = max(round(2 * feeAmount / 100), 1) used elsewhere in the SDK. + // The previous fallback (`|| TICK_SPACINGS[FeeAmount.MEDIUM]`) silently coerced + // every unrecognized fee tier to 60, which caused legacy links to non-standard + // pools (e.g. feeTier=1000) to land on the create-position page with a wrong + // tick spacing, fracturing liquidity into a parallel non-existent pool. + const tickSpacing = TICK_SPACINGS[feeTierNumber as FeeAmount] ?? calculateTickSpacingFromFeeAmount(feeTierNumber) updates.fee = { feeAmount: feeTierNumber,