From 404e95acb68c56e98659d2e1aa5d48b9c8cecb45 Mon Sep 17 00:00:00 2001 From: Tyrone Johnson Date: Thu, 6 Aug 2026 17:45:56 +0300 Subject: [PATCH 1/3] fix(sdk): Ambient route slippage divided by 1000 instead of 10000 The smart-router passes slippage in basis points (10_000 = 100%, documented in internal/utils.getMinReturn and constants.ts), and sdex's CrocSwapPlan consumes a plain fraction (0.01 = 1%). The Ambient route divided the bps value by 1000, so the slippage bound encoded into BOB swap calldata was 10x looser than the tolerance the user selected and the UI displayed as 'Minimum received' (default 0.5% enforced as 5%; ~10.25% on multi-hop routes because the bound is applied to a Q64.64 sqrt price). Reported in advisory GHSA-jx33-xg6c-px39. Divide by 10000 and use the shared DEFAULT_SWAP_SLIPPAGE constant as the fallback (1%, consistent with getMinReturn's default) instead of a bare literal. Add route-level regression tests that capture the fraction handed to sdex and assert it matches getMinReturn's basis-point semantics across tolerance values. --- .changeset/ambient-slippage-basis-points.md | 5 + .../_tests/swaps/routes/ambient-route.test.ts | 94 +++++++++++++++++++ .../src/swaps/smart-router/routes/ambient.ts | 5 +- 3 files changed, 103 insertions(+), 1 deletion(-) create mode 100644 .changeset/ambient-slippage-basis-points.md create mode 100644 packages/sdk/src/_tests/swaps/routes/ambient-route.test.ts diff --git a/.changeset/ambient-slippage-basis-points.md b/.changeset/ambient-slippage-basis-points.md new file mode 100644 index 000000000..d01d3ba72 --- /dev/null +++ b/.changeset/ambient-slippage-basis-points.md @@ -0,0 +1,5 @@ +--- +'@sovryn/sdk': patch +--- + +Fix Ambient route slippage encoding: `options.slippage` is expressed in basis points (10_000 = 100%) but was divided by 1000 instead of 10000 before being passed to sdex, so the enforced slippage bound was 10x looser than the value the user selected (e.g. 0.5% became 5%). The fallback default now uses the shared `DEFAULT_SWAP_SLIPPAGE` constant (1%). diff --git a/packages/sdk/src/_tests/swaps/routes/ambient-route.test.ts b/packages/sdk/src/_tests/swaps/routes/ambient-route.test.ts new file mode 100644 index 000000000..5cf98494a --- /dev/null +++ b/packages/sdk/src/_tests/swaps/routes/ambient-route.test.ts @@ -0,0 +1,94 @@ +import { BigNumber, constants, providers } from 'ethers'; + +import { DEFAULT_SWAP_SLIPPAGE } from '../../../constants'; +import { getMinReturn } from '../../../internal/utils'; +import { ambientRoute } from '../../../swaps/smart-router/routes/ambient'; +import { SwapRoute } from '../../../swaps/smart-router/types'; + +const BOB_MAINNET_CHAIN_ID = 60808; + +const mockTokenA = '0x00000000000000000000000000000000000000aa'; +const mockTokenB = '0x00000000000000000000000000000000000000bb'; +const mockZeroAddress = '0x0000000000000000000000000000000000000000'; + +// captures the slippage options handed to sdex's CrocEnv swap plans +const mockCapturedSlippages: number[] = []; + +jest.mock('@sovryn/sdex', () => ({ + CrocEnv: jest.fn().mockImplementation(() => ({ + context: Promise.resolve({ + dex: { address: mockZeroAddress }, + chain: { proxyPaths: { long: 0 } }, + }), + tokens: { + materialize: () => ({ decimals: 18 }), + }, + sell: () => ({ + for: (_destination: string, opts: { slippage?: number }) => { + if (opts.slippage !== undefined) { + mockCapturedSlippages.push(opts.slippage); + } + return { + generateSwapData: async () => ({ + to: mockZeroAddress, + data: '0x', + value: '0', + }), + }; + }, + }), + })), +})); + +jest.mock('../../../swaps/smart-router/utils/ambient-utils', () => ({ + ...jest.requireActual('../../../swaps/smart-router/utils/ambient-utils'), + fetchPools: async () => [[mockTokenA, mockTokenB, 400]], +})); + +describe('Ambient route slippage encoding', () => { + let route: SwapRoute; + + const provider = { + getNetwork: async () => ({ chainId: BOB_MAINNET_CHAIN_ID }), + } as providers.Provider; + + const swapWithSlippage = async (slippage?: number) => { + mockCapturedSlippages.length = 0; + await route.swap( + mockTokenA, + mockTokenB, + constants.WeiPerEther, + constants.AddressZero, + slippage !== undefined ? { slippage } : undefined, + ); + expect(mockCapturedSlippages).toHaveLength(1); + return mockCapturedSlippages[0]; + }; + + beforeEach(() => { + route = ambientRoute(provider); + }); + + it('encodes basis points as the fraction sdex expects (50 bps -> 0.005)', async () => { + const fraction = await swapWithSlippage(50); + expect(fraction).toBeCloseTo(0.005, 10); + }); + + it('defaults to DEFAULT_SWAP_SLIPPAGE when no slippage option is given', async () => { + const fraction = await swapWithSlippage(); + expect(fraction).toBeCloseTo(DEFAULT_SWAP_SLIPPAGE / 10000, 10); + }); + + it('matches getMinReturn basis-point semantics across tolerance values', async () => { + for (const bps of [10, 50, 100, 1000]) { + const fraction = await swapWithSlippage(bps); + + const minReturn = getMinReturn(constants.WeiPerEther, bps); + const impliedMinReturn = BigNumber.from( + BigInt(Math.round((1 - fraction) * 1e18)).toString(), + ); + + expect(impliedMinReturn.toString()).toEqual(minReturn.toString()); + } + }); +}); diff --git a/packages/sdk/src/swaps/smart-router/routes/ambient.ts b/packages/sdk/src/swaps/smart-router/routes/ambient.ts index fb5f2a999..5556074c9 100644 --- a/packages/sdk/src/swaps/smart-router/routes/ambient.ts +++ b/packages/sdk/src/swaps/smart-router/routes/ambient.ts @@ -6,6 +6,7 @@ import { CrocEnv, CrocPoolView } from '@sovryn/sdex'; import { OrderDirective } from '@sovryn/sdex/dist/encoding/longform'; import { Decimal } from '@sovryn/utils'; +import { DEFAULT_SWAP_SLIPPAGE } from '../../../constants'; import { SovrynErrorCode, makeError } from '../../../errors/errors'; import { hasEnoughAllowance, @@ -230,7 +231,9 @@ export const ambientRoute: SwapRouteFunction = ( }, permit: async () => Promise.resolve(undefined), swap: async (entry, destination, amount, from, options, overrides) => { - const slippage = Number(options?.slippage ?? 50) / 1000; + // options.slippage is in basis points (10_000 = 100%); sdex expects a fraction + const slippage = + Number(options?.slippage ?? DEFAULT_SWAP_SLIPPAGE) / 10000; const pools = await loadPools(); const chainId = await getChainId(); From 05e7af1097cd73bc1f91e59d2ce8d1a6ce482cd8 Mon Sep 17 00:00:00 2001 From: Tyrone Johnson Date: Thu, 6 Aug 2026 18:05:57 +0300 Subject: [PATCH 2/3] test(sdk): pin multi-hop Ambient sqrt-price slippage bound MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extend the slippage-encoding suite to the long-form multi-hop path: drive an A->B->C swap through the real OrderDirective encoder (spied), with calcImpact mocked to a unit Q64.64 sqrt price, and assert each pool's encoded limitPrice is finalPrice * (1 +/- 0.005) for 50 bps — buy and sell legs. Verified to fail (1 +/- 0.05) when the old /1000 divisor is reintroduced. ambient.ts coverage 21% -> 54%; all remaining uncovered lines are outside the slippage flow. --- .../_tests/swaps/routes/ambient-route.test.ts | 67 ++++++++++++++++++- 1 file changed, 64 insertions(+), 3 deletions(-) diff --git a/packages/sdk/src/_tests/swaps/routes/ambient-route.test.ts b/packages/sdk/src/_tests/swaps/routes/ambient-route.test.ts index 5cf98494a..b0e85f468 100644 --- a/packages/sdk/src/_tests/swaps/routes/ambient-route.test.ts +++ b/packages/sdk/src/_tests/swaps/routes/ambient-route.test.ts @@ -1,5 +1,7 @@ import { BigNumber, constants, providers } from 'ethers'; +import { OrderDirective } from '@sovryn/sdex/dist/encoding/longform'; + import { DEFAULT_SWAP_SLIPPAGE } from '../../../constants'; import { getMinReturn } from '../../../internal/utils'; import { ambientRoute } from '../../../swaps/smart-router/routes/ambient'; @@ -9,20 +11,40 @@ const BOB_MAINNET_CHAIN_ID = 60808; const mockTokenA = '0x00000000000000000000000000000000000000aa'; const mockTokenB = '0x00000000000000000000000000000000000000bb'; +// sorts below A and B so the second hop of A -> B -> C is a sell leg +const mockTokenC = '0x0000000000000000000000000000000000000011'; const mockZeroAddress = '0x0000000000000000000000000000000000000000'; +// Q64.64 sqrt price of 1.0, i.e. 2^64 +const mockUnitSqrtPrice = '18446744073709551616'; + +// pools returned by the mocked indexer; tests may override per case +let mockPools: [string, string, number][] = [[mockTokenA, mockTokenB, 400]]; + // captures the slippage options handed to sdex's CrocEnv swap plans const mockCapturedSlippages: number[] = []; jest.mock('@sovryn/sdex', () => ({ CrocEnv: jest.fn().mockImplementation(() => ({ context: Promise.resolve({ - dex: { address: mockZeroAddress }, - chain: { proxyPaths: { long: 0 } }, + dex: { + address: mockZeroAddress, + interface: { encodeFunctionData: () => '0x' }, + }, + chain: { proxyPaths: { long: 130 } }, }), tokens: { materialize: () => ({ decimals: 18 }), }, + pool: async (tokenA: string, tokenB: string, poolIndex: number) => { + const [base, quote] = + tokenA < tokenB ? [tokenA, tokenB] : [tokenB, tokenA]; + return { + baseToken: { tokenAddr: base }, + quoteToken: { tokenAddr: quote }, + poolIndex, + }; + }, sell: () => ({ for: (_destination: string, opts: { slippage?: number }) => { if (opts.slippage !== undefined) { @@ -42,7 +64,12 @@ jest.mock('@sovryn/sdex', () => ({ jest.mock('../../../swaps/smart-router/utils/ambient-utils', () => ({ ...jest.requireActual('../../../swaps/smart-router/utils/ambient-utils'), - fetchPools: async () => [[mockTokenA, mockTokenB, 400]], + fetchPools: async () => mockPools, + calcImpact: async () => ({ + finalPrice: mockUnitSqrtPrice, + baseFlow: '1000000000000000000', + quoteFlow: '-1000000000000000000', + }), })); describe('Ambient route slippage encoding', () => { @@ -66,6 +93,7 @@ describe('Ambient route slippage encoding', () => { }; beforeEach(() => { + mockPools = [[mockTokenA, mockTokenB, 400]]; route = ambientRoute(provider); }); @@ -91,4 +119,37 @@ describe('Ambient route slippage encoding', () => { expect(impliedMinReturn.toString()).toEqual(minReturn.toString()); } }); + + it('bounds each multi-hop pool sqrt price by the same fraction (50 bps -> 1 +/- 0.005)', async () => { + mockPools = [ + [mockTokenA, mockTokenB, 400], + [mockTokenC, mockTokenB, 410], + ]; + route = ambientRoute(provider); + + const appendPoolSpy = jest.spyOn(OrderDirective.prototype, 'appendPool'); + + // no direct A/C pool -> long-form multi-hop path A -> B -> C + await route.swap( + mockTokenA, + mockTokenC, + constants.WeiPerEther, + constants.AddressZero, + { slippage: 50 }, + ); + + const orderPools = appendPoolSpy.mock.results.map(result => result.value); + appendPoolSpy.mockRestore(); + expect(orderPools).toHaveLength(2); + + // calcImpact is mocked to a sqrt price of exactly 1.0 (2^64), so the + // encoded limitPrice ratio is the applied sqrt-price slippage bound + const boundRatios = orderPools.map( + pool => Number(pool.swap.limitPrice.toString()) / 2 ** 64, + ); + + // first hop buys (entry is base), second hop sells (entry is quote) + expect(boundRatios[0]).toBeCloseTo(1.005, 6); + expect(boundRatios[1]).toBeCloseTo(0.995, 6); + }); }); From c690e2f8233431730080d3a6074098560adc360d Mon Sep 17 00:00:00 2001 From: Tyrone Johnson Date: Thu, 6 Aug 2026 18:20:36 +0300 Subject: [PATCH 3/3] fix(sdk): keep Ambient fallback slippage at 50 bps (0.5%) Decision (Tyrone): the UI preset tolerance is 0.5% on both Sovryn AMM and Sovryn DEX on BOB, so the route's no-option fallback keeps the original author's intended 50 bps = 0.5% instead of the SDK-wide DEFAULT_SWAP_SLIPPAGE (1%) introduced in the previous commit. The UI always passes an explicit value; this only affects direct SDK consumers. --- .changeset/ambient-slippage-basis-points.md | 2 +- packages/sdk/src/_tests/swaps/routes/ambient-route.test.ts | 5 ++--- packages/sdk/src/swaps/smart-router/routes/ambient.ts | 7 +++---- 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/.changeset/ambient-slippage-basis-points.md b/.changeset/ambient-slippage-basis-points.md index d01d3ba72..dc6874b17 100644 --- a/.changeset/ambient-slippage-basis-points.md +++ b/.changeset/ambient-slippage-basis-points.md @@ -2,4 +2,4 @@ '@sovryn/sdk': patch --- -Fix Ambient route slippage encoding: `options.slippage` is expressed in basis points (10_000 = 100%) but was divided by 1000 instead of 10000 before being passed to sdex, so the enforced slippage bound was 10x looser than the value the user selected (e.g. 0.5% became 5%). The fallback default now uses the shared `DEFAULT_SWAP_SLIPPAGE` constant (1%). +Fix Ambient route slippage encoding: `options.slippage` is expressed in basis points (10_000 = 100%) but was divided by 1000 instead of 10000 before being passed to sdex, so the enforced slippage bound was 10x looser than the value the user selected (e.g. 0.5% became 5%). The `?? 50` fallback keeps its intended meaning of 0.5%, matching the UI's preset tolerance. diff --git a/packages/sdk/src/_tests/swaps/routes/ambient-route.test.ts b/packages/sdk/src/_tests/swaps/routes/ambient-route.test.ts index b0e85f468..00fffdbf4 100644 --- a/packages/sdk/src/_tests/swaps/routes/ambient-route.test.ts +++ b/packages/sdk/src/_tests/swaps/routes/ambient-route.test.ts @@ -2,7 +2,6 @@ import { BigNumber, constants, providers } from 'ethers'; import { OrderDirective } from '@sovryn/sdex/dist/encoding/longform'; -import { DEFAULT_SWAP_SLIPPAGE } from '../../../constants'; import { getMinReturn } from '../../../internal/utils'; import { ambientRoute } from '../../../swaps/smart-router/routes/ambient'; import { SwapRoute } from '../../../swaps/smart-router/types'; @@ -102,9 +101,9 @@ describe('Ambient route slippage encoding', () => { expect(fraction).toBeCloseTo(0.005, 10); }); - it('defaults to DEFAULT_SWAP_SLIPPAGE when no slippage option is given', async () => { + it('defaults to 50 bps (0.5%, the UI preset) when no slippage option is given', async () => { const fraction = await swapWithSlippage(); - expect(fraction).toBeCloseTo(DEFAULT_SWAP_SLIPPAGE / 10000, 10); + expect(fraction).toBeCloseTo(0.005, 10); }); it('matches getMinReturn basis-point semantics across tolerance values', async () => { diff --git a/packages/sdk/src/swaps/smart-router/routes/ambient.ts b/packages/sdk/src/swaps/smart-router/routes/ambient.ts index 5556074c9..270c2a75b 100644 --- a/packages/sdk/src/swaps/smart-router/routes/ambient.ts +++ b/packages/sdk/src/swaps/smart-router/routes/ambient.ts @@ -6,7 +6,6 @@ import { CrocEnv, CrocPoolView } from '@sovryn/sdex'; import { OrderDirective } from '@sovryn/sdex/dist/encoding/longform'; import { Decimal } from '@sovryn/utils'; -import { DEFAULT_SWAP_SLIPPAGE } from '../../../constants'; import { SovrynErrorCode, makeError } from '../../../errors/errors'; import { hasEnoughAllowance, @@ -231,9 +230,9 @@ export const ambientRoute: SwapRouteFunction = ( }, permit: async () => Promise.resolve(undefined), swap: async (entry, destination, amount, from, options, overrides) => { - // options.slippage is in basis points (10_000 = 100%); sdex expects a fraction - const slippage = - Number(options?.slippage ?? DEFAULT_SWAP_SLIPPAGE) / 10000; + // options.slippage is in basis points (10_000 = 100%); sdex expects a + // fraction. Fallback 50 bps = 0.5%, the UI's preset tolerance. + const slippage = Number(options?.slippage ?? 50) / 10000; const pools = await loadPools(); const chainId = await getChainId();