fix(v4-sdk): swap function accepts pools with all hooks except swap h…

…ooks (#195)

Co-authored-by: Siyu Jiang (See-You John) <[email protected]>

sdks/v4-sdk/src/entities/pool.test.ts

@@ -11,6 +11,8 @@ import {
   ONE_ETHER,
   TICK_SPACING_TEN,
 } from '../internalConstants'
+import { constructHookAddress } from '../utils/hook.test'
+import { HookOptions } from '../utils/hook'
 
 describe('Pool', () => {
   const USDC = new Token(1, '0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48', 6, 'USDC', 'USD Coin')
@@ -261,6 +263,7 @@ describe('Pool', () => {
 
   describe('swaps', () => {
     let pool: Pool
+    let poolWithSwapHook: Pool
 
     beforeEach(() => {
       pool = new Pool(
@@ -285,9 +288,26 @@ describe('Pool', () => {
           },
         ]
       )
+
+      poolWithSwapHook = new Pool(
+        USDC,
+        DAI,
+        FEE_AMOUNT_LOW,
+        TICK_SPACING_TEN,
+        constructHookAddress([HookOptions.BeforeSwap]),
+        encodeSqrtRatioX96(1, 1),
+        ONE_ETHER,
+        0,
+        []
+      )
     })
 
     describe('#getOutputAmount', () => {
+      it('throws if pool has beforeSwap hooks', async () => {
+        const inputAmount = CurrencyAmount.fromRawAmount(USDC, 100)
+        await expect(() => poolWithSwapHook.getOutputAmount(inputAmount)).rejects.toThrow('Unsupported hook')
+      })
+
       it('USDC -> DAI', async () => {
         const inputAmount = CurrencyAmount.fromRawAmount(USDC, 100)
         const [outputAmount] = await pool.getOutputAmount(inputAmount)
@@ -304,6 +324,11 @@ describe('Pool', () => {
     })
 
     describe('#getInputAmount', () => {
+      it('throws if pool has beforeSwap hooks', async () => {
+        const outputAmount = CurrencyAmount.fromRawAmount(DAI, 98)
+        await expect(() => poolWithSwapHook.getInputAmount(outputAmount)).rejects.toThrow('Unsupported hook')
+      })
+
       it('USDC -> DAI', async () => {
         const outputAmount = CurrencyAmount.fromRawAmount(DAI, 98)
         const [inputAmount] = await pool.getInputAmount(outputAmount)

sdks/v4-sdk/src/entities/pool.ts

@@ -12,6 +12,7 @@ import {
 } from '@uniswap/v3-sdk'
 import { defaultAbiCoder, isAddress } from 'ethers/lib/utils'
 import { sortsBefore } from '../utils/sortsBefore'
+import { Hook } from '../utils/hook'
 import { ADDRESS_ZERO, NEGATIVE_ONE, Q192 } from '../internalConstants'
 import JSBI from 'jsbi'
 
@@ -299,7 +300,7 @@ export class Pool {
     amountSpecified: JSBI,
     sqrtPriceLimitX96?: JSBI
   ): Promise<{ amountCalculated: JSBI; sqrtRatioX96: JSBI; liquidity: JSBI; tickCurrent: number }> {
-    if (this.nonImpactfulHook()) {
+    if (!this.hookImpactsSwap()) {
       return v3Swap(
         JSBI.BigInt(this.fee),
         this.sqrtRatioX96,
@@ -316,8 +317,9 @@ export class Pool {
     }
   }
 
-  private nonImpactfulHook(): boolean {
-    // TODO: reference chain specific hook addresses or patterns that do not impact swaps
-    return this.hooks === ADDRESS_ZERO
+  private hookImpactsSwap(): boolean {
+    // could use this function to clear certain hooks that may have swap Permissions, but we know they don't interfere
+    // in the swap outcome
+    return Hook.hasSwapPermissions(this.hooks)
   }
 }

sdks/v4-sdk/src/index.ts

@@ -1,4 +1,3 @@
 export * from './entities'
 export * from './utils'
 export * from './PositionManager'
-export * from './hook'

sdks/v4-sdk/src/hook.test.ts → sdks/v4-sdk/src/utils/hook.test.ts

@@ -1,6 +1,6 @@
 import { Hook, HookOptions, hookFlagIndex } from './hook'
 
-function constructAddress(hookOptions: HookOptions[]): string {
+export function constructHookAddress(hookOptions: HookOptions[]): string {
   let hookFlags = 0
   for (const hookOption of hookOptions) {
     hookFlags = hookFlags | (1 << hookFlagIndex[hookOption])
@@ -13,20 +13,20 @@ function constructAddress(hookOptions: HookOptions[]): string {
 describe('Hook', () => {
   const allHooksAddress = '0x0000000000000000000000000000000000003fff'
   const emptyHookAddress = '0x0000000000000000000000000000000000000000'
-  const hookBeforeInitialize = constructAddress([HookOptions.BeforeInitialize])
-  const hookAfterInitialize = constructAddress([HookOptions.AfterInitialize])
-  const hookBeforeAddLiquidity = constructAddress([HookOptions.BeforeAddLiquidity])
-  const hookAfterAddLiquidity = constructAddress([HookOptions.AfterAddLiquidity])
-  const hookBeforeRemoveLiquidity = constructAddress([HookOptions.BeforeRemoveLiquidity])
-  const hookAfterRemoveLiquidity = constructAddress([HookOptions.AfterRemoveLiquidity])
-  const hookBeforeSwap = constructAddress([HookOptions.BeforeSwap])
-  const hookAfterSwap = constructAddress([HookOptions.AfterSwap])
-  const hookBeforeDonate = constructAddress([HookOptions.BeforeDonate])
-  const hookAfterDonate = constructAddress([HookOptions.AfterDonate])
-  const hookBeforeSwapReturnsDelta = constructAddress([HookOptions.BeforeSwapReturnsDelta])
-  const hookAfterSwapReturnsDelta = constructAddress([HookOptions.AfterSwapReturnsDelta])
-  const hookAfterAddLiquidityReturnsDelta = constructAddress([HookOptions.AfterAddLiquidityReturnsDelta])
-  const hookAfterRemoveLiquidityReturnsDelta = constructAddress([HookOptions.AfterRemoveLiquidityReturnsDelta])
+  const hookBeforeInitialize = constructHookAddress([HookOptions.BeforeInitialize])
+  const hookAfterInitialize = constructHookAddress([HookOptions.AfterInitialize])
+  const hookBeforeAddLiquidity = constructHookAddress([HookOptions.BeforeAddLiquidity])
+  const hookAfterAddLiquidity = constructHookAddress([HookOptions.AfterAddLiquidity])
+  const hookBeforeRemoveLiquidity = constructHookAddress([HookOptions.BeforeRemoveLiquidity])
+  const hookAfterRemoveLiquidity = constructHookAddress([HookOptions.AfterRemoveLiquidity])
+  const hookBeforeSwap = constructHookAddress([HookOptions.BeforeSwap])
+  const hookAfterSwap = constructHookAddress([HookOptions.AfterSwap])
+  const hookBeforeDonate = constructHookAddress([HookOptions.BeforeDonate])
+  const hookAfterDonate = constructHookAddress([HookOptions.AfterDonate])
+  const hookBeforeSwapReturnsDelta = constructHookAddress([HookOptions.BeforeSwapReturnsDelta])
+  const hookAfterSwapReturnsDelta = constructHookAddress([HookOptions.AfterSwapReturnsDelta])
+  const hookAfterAddLiquidityReturnsDelta = constructHookAddress([HookOptions.AfterAddLiquidityReturnsDelta])
+  const hookAfterRemoveLiquidityReturnsDelta = constructHookAddress([HookOptions.AfterRemoveLiquidityReturnsDelta])
 
   describe('permissions', () => {
     it('throws for an invalid address', () => {
@@ -236,4 +236,68 @@ describe('Hook', () => {
       ).toEqual(false)
     })
   })
+
+  describe('hasInitializePermissions', () => {
+    it('returns the correct results for beforeSwap', () => {
+      expect(Hook.hasInitializePermissions(hookBeforeInitialize)).toEqual(true)
+    })
+
+    it('returns the correct results for afterInitialize', () => {
+      expect(Hook.hasInitializePermissions(hookAfterInitialize)).toEqual(true)
+    })
+
+    it('returns false for non-donate hooks', () => {
+      expect(Hook.hasInitializePermissions(hookAfterSwap)).toEqual(false)
+    })
+  })
+
+  describe('hasLiquidityPermissions', () => {
+    it('returns the correct results for beforeAddLiquidity', () => {
+      expect(Hook.hasLiquidityPermissions(hookBeforeAddLiquidity)).toEqual(true)
+    })
+
+    it('returns the correct results for afterAddLiquidity', () => {
+      expect(Hook.hasLiquidityPermissions(hookAfterAddLiquidity)).toEqual(true)
+    })
+
+    it('returns the correct results for beforeRemoveLiquidity', () => {
+      expect(Hook.hasLiquidityPermissions(hookBeforeRemoveLiquidity)).toEqual(true)
+    })
+
+    it('returns the correct results for afterRemoveLiquidity', () => {
+      expect(Hook.hasLiquidityPermissions(hookAfterRemoveLiquidity)).toEqual(true)
+    })
+
+    it('returns false if only delta flag is flagged (an incorrect address)', () => {
+      expect(Hook.hasLiquidityPermissions(hookAfterRemoveLiquidityReturnsDelta)).toEqual(false)
+    })
+  })
+
+  describe('hasSwapPermissions', () => {
+    it('returns the correct results for beforeSwap', () => {
+      expect(Hook.hasSwapPermissions(hookBeforeSwap)).toEqual(true)
+    })
+
+    it('returns the correct results for afterSwap', () => {
+      expect(Hook.hasSwapPermissions(hookAfterSwap)).toEqual(true)
+    })
+
+    it('returns false if only delta flag is flagged (an incorrect address)', () => {
+      expect(Hook.hasSwapPermissions(hookBeforeSwapReturnsDelta)).toEqual(false)
+    })
+  })
+
+  describe('hasDonatePermissions', () => {
+    it('returns the correct results for beforeSwap', () => {
+      expect(Hook.hasDonatePermissions(hookBeforeDonate)).toEqual(true)
+    })
+
+    it('returns the correct results for afterDonate', () => {
+      expect(Hook.hasDonatePermissions(hookAfterDonate)).toEqual(true)
+    })
+
+    it('returns false for non-donate hooks', () => {
+      expect(Hook.hasDonatePermissions(hookAfterSwap)).toEqual(false)
+    })
+  })
 })

sdks/v4-sdk/src/hook.ts → sdks/v4-sdk/src/utils/hook.ts

@@ -39,7 +39,7 @@ export const hookFlagIndex = {
 
 export class Hook {
   public static permissions(address: string): HookPermissions {
-    invariant(isAddress(address), 'invalid address')
+    this._checkAddress(address)
     return {
       beforeInitialize: this._hasPermission(address, HookOptions.BeforeInitialize),
       afterInitialize: this._hasPermission(address, HookOptions.AfterInitialize),
@@ -59,11 +59,47 @@ export class Hook {
   }
 
   public static hasPermission(address: string, hookOption: HookOptions) {
-    invariant(isAddress(address), 'invalid address')
+    this._checkAddress(address)
     return this._hasPermission(address, hookOption)
   }
 
+  public static hasInitializePermissions(address: string) {
+    this._checkAddress(address)
+    return (
+      this._hasPermission(address, HookOptions.BeforeInitialize) ||
+      Hook._hasPermission(address, HookOptions.AfterInitialize)
+    )
+  }
+
+  public static hasLiquidityPermissions(address: string) {
+    this._checkAddress(address)
+    // this implicitly encapsulates liquidity delta permissions
+    return (
+      this._hasPermission(address, HookOptions.BeforeAddLiquidity) ||
+      Hook._hasPermission(address, HookOptions.AfterAddLiquidity) ||
+      Hook._hasPermission(address, HookOptions.BeforeRemoveLiquidity) ||
+      Hook._hasPermission(address, HookOptions.AfterRemoveLiquidity)
+    )
+  }
+
+  public static hasSwapPermissions(address: string) {
+    this._checkAddress(address)
+    // this implicitly encapsulates swap delta permissions
+    return this._hasPermission(address, HookOptions.BeforeSwap) || Hook._hasPermission(address, HookOptions.AfterSwap)
+  }
+
+  public static hasDonatePermissions(address: string) {
+    this._checkAddress(address)
+    return (
+      this._hasPermission(address, HookOptions.BeforeDonate) || Hook._hasPermission(address, HookOptions.AfterDonate)
+    )
+  }
+
   private static _hasPermission(address: string, hookOption: HookOptions) {
     return !!(parseInt(address, 16) & (1 << hookFlagIndex[hookOption]))
   }
+
+  private static _checkAddress(address: string) {
+    invariant(isAddress(address), 'invalid address')
+  }
 }

sdks/v4-sdk/src/utils/index.ts

@@ -8,3 +8,4 @@ export * from './encodeRouteToPath'
 export * from './pathCurrency'
 export * from './priceTickConversions'
 export * from './sortsBefore'
+export * from './hook'