Merge pull request #526 from balancer/fix-price-impact-nested-pools

Fix price impact with near proportional inputs for AddLiquidityNested

.changeset/chilled-phones-explain.md

@@ -0,0 +1,5 @@
+---
+"@balancer/sdk": patch
+---
+
+Fix price impact with near proportional inputs for AddLiquidityNested

src/entities/priceImpact/addLiquidityUnbalancedBoosted.ts

@@ -82,33 +82,16 @@ export async function addLiquidityUnbalancedBoosted(
         if (deltas[i] === 0n) {
             deltaBPTs.push(0n);
         } else {
-            try {
-                deltaBPTs.push(
-                    await queryAddLiquidityForTokenDelta(
-                        addLiquidity,
-                        input,
-                        poolState,
-                        poolTokens,
-                        i,
-                        deltas[i],
-                    ),
-                );
-            } catch (err) {
-                // see https://viem.sh/docs/contract/simulateContract#handling-custom-errors
-                if (err instanceof BaseError) {
-                    const revertError = err.walk(
-                        (err) => err instanceof ContractFunctionRevertedError,
-                    );
-                    if (revertError instanceof ContractFunctionRevertedError) {
-                        const errorName = revertError.data?.errorName ?? '';
-                        if (errorName === 'WrapAmountTooSmall') {
-                            deltaBPTs.push(0n);
-                            continue;
-                        }
-                    }
-                }
-                throw err;
-            }
+            deltaBPTs.push(
+                await queryAddLiquidityForTokenDelta(
+                    addLiquidity,
+                    input,
+                    poolState,
+                    poolTokens,
+                    i,
+                    deltas[i],
+                ),
+            );
         }
     }
 
@@ -166,6 +149,18 @@ async function queryAddLiquidityForTokenDelta(
         );
         return delta < 0n ? -deltaBPT.amount : deltaBPT.amount;
     } catch (err) {
+        // see https://viem.sh/docs/contract/simulateContract#handling-custom-errors
+        if (err instanceof BaseError) {
+            const revertError = err.walk(
+                (err) => err instanceof ContractFunctionRevertedError,
+            );
+            if (revertError instanceof ContractFunctionRevertedError) {
+                const errorName = revertError.data?.errorName ?? '';
+                if (errorName === 'WrapAmountTooSmall') {
+                    return 0n;
+                }
+            }
+        }
         throw new Error(
             `Unexpected error while calculating addLiquidityUnbalancedBoosted PI at Delta add step:\n${err}`,
         );

test/anvil/anvil-global-setup.ts

@@ -66,7 +66,7 @@ export const ANVIL_NETWORKS: Record<NetworksWithFork, NetworkSetup> = {
         rpcEnv: 'SEPOLIA_RPC_URL',
         fallBackRpc: 'https://sepolia.gateway.tenderly.co',
         port: ANVIL_PORTS.SEPOLIA,
-        forkBlockNumber: 7220370n,
+        forkBlockNumber: 7245070n,
     },
     OPTIMISM: {
         rpcEnv: 'OPTIMISM_RPC_URL',

test/lib/utils/addLiquidityHelper.ts

@@ -124,7 +124,7 @@ function getCheck(output: AddLiquidityQueryOutput) {
     const kind = output.addLiquidityKind;
     switch (kind) {
         case AddLiquidityKind.Proportional:
-            return { ...check, bptOut, amountsIn };
+            return { ...check };
         case AddLiquidityKind.SingleToken:
             return { ...check, bptOut };
         case AddLiquidityKind.Unbalanced:
@@ -355,26 +355,6 @@ export function assertAddLiquidityProportional(
     const { txOutput, addLiquidityQueryOutput, addLiquidityBuildCallOutput } =
         addLiquidityOutput;
 
-    // replace referenceAmount into amountsIn or bptOut for comparison
-    const bptOut = addLiquidityQueryOutput.bptOut.token.isSameAddress(
-        addLiquidityInput.referenceAmount.address,
-    )
-        ? TokenAmount.fromRawAmount(
-              addLiquidityQueryOutput.bptOut.token,
-              addLiquidityInput.referenceAmount.rawAmount,
-          )
-        : addLiquidityQueryOutput.bptOut;
-    const amountsIn = addLiquidityOutput.addLiquidityQueryOutput.amountsIn.map(
-        (a) =>
-            a.token.isSameAddress(addLiquidityInput.referenceAmount.address) &&
-            !a.token.isSameAddress(poolState.address) // skip CSP BPT as token
-                ? TokenAmount.fromRawAmount(
-                      a.token,
-                      addLiquidityInput.referenceAmount.rawAmount,
-                  )
-                : a,
-    );
-
     let to: Address;
 
     switch (protocolVersion) {
@@ -391,15 +371,13 @@ export function assertAddLiquidityProportional(
             throw new Error(`Unsupported protocolVersion: ${protocolVersion}`);
     }
 
-    let expectedQueryOutput:
-        | Omit<AddLiquidityQueryOutput, 'bptIndex'>
-        | (Omit<AddLiquidityQueryOutput, 'bptOut' | 'bptIndex'> & {
-              userData: Hex;
-          }) = {
+    let expectedQueryOutput: Omit<
+        AddLiquidityQueryOutput,
+        'bptOut' | 'bptIndex' | 'amountsIn'
+    > & {
+        userData?: Hex;
+    } = {
         to,
-        // Query should use same referenceAmount as user sets
-        bptOut,
-        amountsIn,
         // Only expect tokenInIndex for AddLiquiditySingleToken
         tokenInIndex: undefined,
         // Should match inputs
@@ -417,6 +395,29 @@ export function assertAddLiquidityProportional(
 
     expect(queryCheck).to.deep.eq(expectedQueryOutput);
 
+    if (
+        addLiquidityQueryOutput.bptOut.token.isSameAddress(
+            addLiquidityInput.referenceAmount.address,
+        )
+    ) {
+        // referenceAmount as bptOut - queryOutput should be an exact match with user provided input
+        expect(addLiquidityQueryOutput.bptOut.amount).toEqual(
+            addLiquidityInput.referenceAmount.rawAmount,
+        );
+    } else {
+        // referenceAmount as amountsIn - queryOutput can be 1 wei diff from user provided amount
+        addLiquidityQueryOutput.amountsIn.forEach((a) => {
+            if (
+                a.token.isSameAddress(addLiquidityInput.referenceAmount.address)
+            )
+                expect(
+                    Number(
+                        a.amount - addLiquidityInput.referenceAmount.rawAmount,
+                    ),
+                ).closeTo(0, 1);
+        });
+    }
+
     // Expect all assets in to have an amount > 0 apart from BPT if it exists
     addLiquidityQueryOutput.amountsIn.forEach((a) => {
         if (a.token.address === poolState.address) expect(a.amount).toEqual(0n);

test/mockData/partialBoostedPool.ts

@@ -1,8 +1,8 @@
 import { PoolStateWithUnderlyings } from '@/entities';
 
 export const partialBoostedPool_USDT_stataDAI: PoolStateWithUnderlyings = {
-    id: '0xCE7601b157e0871332D2295F274a0f4314a1585D',
-    address: '0xCE7601b157e0871332D2295F274a0f4314a1585D',
+    id: '0x070810362cb6fd4b44f87a225ab0c20aeb194a63',
+    address: '0x070810362cb6fd4b44f87a225ab0c20aeb194a63',
     type: 'Stable',
     protocolVersion: 3,
     tokens: [

test/v3/priceImpact/priceImpact.V3.integration.test.ts

@@ -25,19 +25,17 @@ const USDT = TOKENS[chainId].USDT_AAVE;
 const DAI = TOKENS[chainId].DAI_AAVE;
 const WETH = TOKENS[chainId].WETH;
 
+/**
+ * FIXME: tests are here just as a sanity check. We should find a way to
+ * properly validate results.
+ */
 describe('PriceImpact V3', () => {
     let rpcUrl: string;
     beforeAll(async () => {
         ({ rpcUrl } = await startFork(ANVIL_NETWORKS.SEPOLIA));
     });
-    /**
-     * FIXME: Test pending a reference value for comparison/validation, because
-     * there is no corresponding method in previous SDK to validate the result.
-     * We should be able to infer that it is correct because it follows the same
-     * ABA approach as price impact for other actions (addLiquidity, swap, etc.)
-     */
     describe('Full Boosted Pool Boosted Pool AddLiquidity', () => {
-        test.skip('Close to proportional', async () => {
+        test('Close to proportional', async () => {
             const addLiquidityInput: AddLiquidityBoostedUnbalancedInput = {
                 chainId,
                 rpcUrl,
@@ -61,7 +59,7 @@ describe('PriceImpact V3', () => {
                     addLiquidityInput,
                     boostedPool_USDC_USDT,
                 );
-            const priceImpactSpot = PriceImpactAmount.fromDecimal('0.000057');
+            const priceImpactSpot = PriceImpactAmount.fromDecimal('0.000042');
             expect(priceImpactABA.decimal).eq(priceImpactSpot.decimal);
         });
 
@@ -90,11 +88,11 @@ describe('PriceImpact V3', () => {
                     boostedPool_USDC_USDT,
                 );
             const priceImpactSpot =
-                PriceImpactAmount.fromDecimal('0.00058238995');
+                PriceImpactAmount.fromDecimal('0.0005511004');
             expect(priceImpactABA.decimal).eq(priceImpactSpot.decimal);
         });
 
-        test.skip('Single token input', async () => {
+        test('Single token input', async () => {
             const addLiquidityInput: AddLiquidityBoostedUnbalancedInput = {
                 chainId,
                 rpcUrl,
@@ -113,12 +111,12 @@ describe('PriceImpact V3', () => {
                     addLiquidityInput,
                     boostedPool_USDC_USDT,
                 );
-            const priceImpactSpot = PriceImpactAmount.fromDecimal('0.0004755');
+            const priceImpactSpot = PriceImpactAmount.fromDecimal('0.000492');
             expect(priceImpactABA.decimal).eq(priceImpactSpot.decimal);
         });
     });
 
-    describe.skip('Partial Boosted Pool Boosted Pool AddLiquidity', () => {
+    describe('Partial Boosted Pool Boosted Pool AddLiquidity', () => {
         test('Close to proportional', async () => {
             const addLiquidityInput: AddLiquidityBoostedUnbalancedInput = {
                 chainId,
@@ -143,9 +141,7 @@ describe('PriceImpact V3', () => {
                     addLiquidityInput,
                     partialBoostedPool_USDT_stataDAI,
                 );
-            const priceImpactSpot = PriceImpactAmount.fromDecimal(
-                '0.029220310653853106',
-            );
+            const priceImpactSpot = PriceImpactAmount.fromDecimal('0.0000045');
             expect(priceImpactABA.decimal).eq(priceImpactSpot.decimal);
         });
 
@@ -173,15 +169,13 @@ describe('PriceImpact V3', () => {
                     addLiquidityInput,
                     partialBoostedPool_USDT_stataDAI,
                 );
-            const priceImpactSpot = PriceImpactAmount.fromDecimal(
-                '0.006768518949421069',
-            );
+            const priceImpactSpot =
+                PriceImpactAmount.fromDecimal('0.000396375');
             expect(priceImpactABA.decimal).eq(priceImpactSpot.decimal);
         });
     });
 
-    // FIXME: zeroOutDeltas is swapping a huge amount (~200 WETH) and hitting MaxInRatio
-    describe.skip('Nested pool', () => {
+    describe('Nested pool', () => {
         test('Close to proportional', async () => {
             const addLiquidityInput: AddLiquidityNestedInput = {
                 amountsIn: [
@@ -203,8 +197,9 @@ describe('PriceImpact V3', () => {
                 addLiquidityInput,
                 nestedWithBoostedPool,
             );
-            const priceImpactSpot =
-                PriceImpactAmount.fromDecimal('0.00206614703619');
+            const priceImpactSpot = PriceImpactAmount.fromDecimal(
+                '0.005213821423105922',
+            );
             expect(priceImpactABA.decimal).eq(priceImpactSpot.decimal);
         });
     });