Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(v4-sdk): swap function accepts pools with all hooks except swap hooks #195

Merged
merged 10 commits into from
Nov 14, 2024
Merged
25 changes: 25 additions & 0 deletions sdks/v4-sdk/src/entities/pool.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import {
ONE_ETHER,
TICK_SPACING_TEN,
} from '../internalConstants'
import { constructHookAddress } from '../hook.test'
import { HookOptions } from '../hook'

describe('Pool', () => {
const USDC = new Token(1, '0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48', 6, 'USDC', 'USD Coin')
Expand Down Expand Up @@ -261,6 +263,7 @@ describe('Pool', () => {

describe('swaps', () => {
let pool: Pool
let poolWithSwapHook: Pool

beforeEach(() => {
pool = new Pool(
Expand All @@ -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)
Expand All @@ -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)
Expand Down
10 changes: 6 additions & 4 deletions sdks/v4-sdk/src/entities/pool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
} from '@uniswap/v3-sdk'
import { defaultAbiCoder, isAddress } from 'ethers/lib/utils'
import { sortsBefore } from '../utils/sortsBefore'
import { HookOptions, Hook } from '../hook'
import { ADDRESS_ZERO, NEGATIVE_ONE, Q192 } from '../internalConstants'
import JSBI from 'jsbi'

Expand Down Expand Up @@ -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.hookHasSwapPermissions()) {
return v3Swap(
JSBI.BigInt(this.fee),
this.sqrtRatioX96,
Expand All @@ -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 hookHasSwapPermissions(): boolean {
return (
Hook.hasPermission(this.hooks, HookOptions.BeforeSwap) || Hook.hasPermission(this.hooks, HookOptions.AfterSwap)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need to check BeforeSwapReturnsDelta and AfterSwapReturnsDelta?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, because these options only get hit if either beforeSwap or AfterSwap are true. (before/afterSwapReturnsDelta cannot be true if beforeSwap or afterSwap are false)

https://github.com/Uniswap/v4-core/blob/main/src/libraries/Hooks.sol#L265

Copy link
Member Author

@ewilz ewilz Nov 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can leave a code comment

)
}
}
30 changes: 15 additions & 15 deletions sdks/v4-sdk/src/hook.test.ts
Original file line number Diff line number Diff line change
@@ -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])
Expand All @@ -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', () => {
Expand Down
1 change: 1 addition & 0 deletions sdks/v4-sdk/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
export * from './entities'
export * from './utils'
export * from './PositionManager'
export * from './hook'
Loading