Simplifies types while keeps storing last query response state inside…

… hander (#200)

* Improve add liquidity interface typing

* Fix twamm example in build call data query

* Rename to previewQueryData

lib/modules/pool/actions/add-liquidity/add-liquidity.types.ts

@@ -1,23 +1,11 @@
-import { AddLiquidityQueryOutput, PriceImpact, TokenAmount } from '@balancer/sdk'
+import { TokenAmount } from '@balancer/sdk'
 import { Address } from 'wagmi'
-import { HumanAmountIn } from '../liquidity-types'
 
-// TODO: this type should be exposed by the SDK
-export type PriceImpactAmount = Awaited<ReturnType<typeof PriceImpact.addLiquidityUnbalanced>>
-
-export type AddLiquidityInputs = {
-  humanAmountsIn: HumanAmountIn[]
-  account?: Address
-  slippagePercent?: string
-}
-
-// sdkQueryOutput is optional because it will be only used in cases where we use the SDK to query/build the transaction
-// We will probably need a more abstract interface to be used by edge cases
-export type AddLiquidityOutputs = {
+export type QueryAddLiquidityOutput = {
   bptOut: TokenAmount
-  sdkQueryOutput?: AddLiquidityQueryOutput
 }
 
-export type BuildLiquidityInputs = {
-  inputs: AddLiquidityInputs
+export type BuildAddLiquidityInputs = {
+  account: Address
+  slippagePercent: string
 }

lib/modules/pool/actions/add-liquidity/handlers/AddLiquidity.handler.ts

@@ -1,21 +1,30 @@
 import { TransactionConfig } from '@/lib/modules/web3/contracts/contract.types'
-import {
-  AddLiquidityInputs,
-  AddLiquidityOutputs,
-  BuildLiquidityInputs,
-} from '../add-liquidity.types'
+import { HumanAmountIn } from '../../liquidity-types'
+import { BuildAddLiquidityInputs, QueryAddLiquidityOutput } from '../add-liquidity.types'
 
 /**
  * AddLiquidityHandler is an interface that defines the methods that must be implemented by a handler.
  * They take standard inputs from the UI and return frontend standardised outputs.
- * The outputs should not be return types from the SDK. This is to
- * allow handlers to be developed in the future that may not use the SDK.
+ *
+ * SDK handlers:
+ * - Default handlers will interact with the SDK to query and build the call data for the transaction
+ * - They store the response of the last query execution inside the handler instance so it can be used later by buildAddLiquidityCallData
+ * - Each time queryAddLiquidity is called they update that internal state
+ *
+ * Edge-case handlers:
+ * - Edge case handlers (e.g. Twamm handler) will not interact with the SDK
+ * - They do not store the response of a SDK query but could store other state related to the edge-case implementation
  */
 export interface AddLiquidityHandler {
-  // Query the SDK for the expected output of adding liquidity
-  queryAddLiquidity(inputs: AddLiquidityInputs): Promise<AddLiquidityOutputs>
+  // Query the expected output of adding liquidity and store it inside the handler instance
+  // Also returns bptOut to be used by the UI
+  queryAddLiquidity(humanAmountsIn: HumanAmountIn[]): Promise<QueryAddLiquidityOutput>
   // Calculate the price impact of adding liquidity
-  calculatePriceImpact(inputs: AddLiquidityInputs): Promise<number>
-  // Build tx payload for adding liquidity
-  buildAddLiquidityCallData(inputs: BuildLiquidityInputs): Promise<TransactionConfig>
+  calculatePriceImpact(humanAmountsIn: HumanAmountIn[]): Promise<number>
+  /*
+    Build tx callData payload for adding liquidity
+    It is responsibility of the UI to avoid calling buildAddLiquidityCallData before the last queryAddLiquidity was finished
+    ("Next" button will be disabled if the react-query calling queryAddLiquidity is in loading state)
+  */
+  buildAddLiquidityCallData(inputs: BuildAddLiquidityInputs): Promise<TransactionConfig>
 }

lib/modules/pool/actions/add-liquidity/handlers/TwammAddLiquidity.handler.ts

@@ -2,11 +2,8 @@ import { SupportedChainId } from '@/lib/config/config.types'
 import { TransactionConfig } from '@/lib/modules/web3/contracts/contract.types'
 import { emptyAddress } from '@/lib/modules/web3/contracts/wagmi-helpers'
 import { Token, TokenAmount } from '@balancer/sdk'
-import {
-  AddLiquidityInputs,
-  AddLiquidityOutputs,
-  BuildLiquidityInputs,
-} from '../add-liquidity.types'
+import { HumanAmountIn } from '../../liquidity-types'
+import { BuildAddLiquidityInputs } from '../add-liquidity.types'
 import { AddLiquidityHandler } from './AddLiquidity.handler'
 
 /**
@@ -15,12 +12,13 @@ import { AddLiquidityHandler } from './AddLiquidity.handler'
  * This is just a fake example to show how to implement edge-case handlers.
  */
 export class TwammAddLiquidityHandler implements AddLiquidityHandler {
+  humanAmountsIn?: HumanAmountIn[]
+
   constructor(private chainId: SupportedChainId) {}
 
   // TODO: This is a non-sense example implementation
-  public async queryAddLiquidity({
-    humanAmountsIn,
-  }: AddLiquidityInputs): Promise<AddLiquidityOutputs> {
+  public async queryAddLiquidity(humanAmountsIn: HumanAmountIn[]) {
+    this.humanAmountsIn = humanAmountsIn
     const tokenAmount = TokenAmount.fromHumanAmount(
       {} as unknown as Token,
       humanAmountsIn[0].humanAmount || '0'
@@ -30,18 +28,22 @@ export class TwammAddLiquidityHandler implements AddLiquidityHandler {
   }
 
   // TODO: This is a non-sense example implementation
-  public async calculatePriceImpact({ humanAmountsIn }: AddLiquidityInputs): Promise<number> {
+  public async calculatePriceImpact(humanAmountsIn: HumanAmountIn[]): Promise<number> {
     return Number(humanAmountsIn[0].humanAmount)
   }
 
   // TODO: This is a non-sense example implementation
-  public async buildAddLiquidityCallData(
-    buildInputs: BuildLiquidityInputs
-  ): Promise<TransactionConfig> {
-    const { humanAmountsIn, account, slippagePercent } = buildInputs.inputs
-    if (!account || !slippagePercent) throw new Error('Missing account or slippage')
+  public async buildAddLiquidityCallData({
+    account,
+  }: BuildAddLiquidityInputs): Promise<TransactionConfig> {
+    if (!this.humanAmountsIn) {
+      throw new Error(
+        `Missing humanAmountsIn.
+This probably means that you tried to run build callData before running queryAddLiquidity`
+      )
+    }
 
-    const value = BigInt(humanAmountsIn[0].humanAmount)
+    const value = BigInt(this.humanAmountsIn[0].humanAmount)
 
     return {
       account,

lib/modules/pool/actions/add-liquidity/handlers/UnbalancedAddLiquidity.handler.integration.spec.ts

@@ -1,3 +1,4 @@
+/* eslint-disable max-len */
 import networkConfig from '@/lib/config/networks/mainnet'
 import { balAddress, wETHAddress, wjAuraAddress } from '@/lib/debug-helpers'
 import { aWjAuraWethPoolElementMock } from '@/test/msw/builders/gqlPoolElement.builders'
@@ -6,12 +7,12 @@ import { HumanAmount } from '@balancer/sdk'
 import { Address } from 'viem'
 import { aPhantomStablePoolStateInputMock } from '../../../__mocks__/pool.builders'
 import { Pool } from '../../../usePool'
-import { selectAddLiquidityHandler } from './selectAddLiquidityHandler'
 import { HumanAmountIn } from '../../liquidity-types'
+import { UnbalancedAddLiquidityHandler } from './UnbalancedAddLiquidity.handler'
+import { selectAddLiquidityHandler } from './selectAddLiquidityHandler'
 
 function selectUnbalancedHandler() {
-  //TODO: refactor mock builders to build poolStateInput and pool at the same time
-  return selectAddLiquidityHandler(aWjAuraWethPoolElementMock())
+  return selectAddLiquidityHandler(aWjAuraWethPoolElementMock()) as UnbalancedAddLiquidityHandler
 }
 
 describe('When adding unbalanced liquidity for a weighted  pool', () => {
@@ -23,7 +24,7 @@ describe('When adding unbalanced liquidity for a weighted  pool', () => {
       { humanAmount: '1', tokenAddress: wjAuraAddress },
     ]
 
-    const priceImpact = await handler.calculatePriceImpact({ humanAmountsIn })
+    const priceImpact = await handler.calculatePriceImpact(humanAmountsIn)
     expect(priceImpact).toBeGreaterThan(0.002)
   })
 
@@ -35,7 +36,7 @@ describe('When adding unbalanced liquidity for a weighted  pool', () => {
       { humanAmount: '', tokenAddress: balAddress },
     ]
 
-    const priceImpact = await handler.calculatePriceImpact({ humanAmountsIn })
+    const priceImpact = await handler.calculatePriceImpact(humanAmountsIn)
 
     expect(priceImpact).toEqual(0)
   })
@@ -48,9 +49,7 @@ describe('When adding unbalanced liquidity for a weighted  pool', () => {
 
     const handler = selectUnbalancedHandler()
 
-    const result = await handler.queryAddLiquidity({
-      humanAmountsIn,
-    })
+    const result = await handler.queryAddLiquidity(humanAmountsIn)
 
     expect(result.bptOut.amount).toBeGreaterThan(300000000000000000000n)
   })
@@ -63,20 +62,43 @@ describe('When adding unbalanced liquidity for a weighted  pool', () => {
 
     const handler = selectUnbalancedHandler()
 
-    await handler.queryAddLiquidity({
-      humanAmountsIn,
-    })
+    // Store query response in handler instance
+    await handler.queryAddLiquidity(humanAmountsIn)
 
-    const inputs = {
-      humanAmountsIn,
+    const result = await handler.buildAddLiquidityCallData({
       account: defaultTestUserAccount,
       slippagePercent: '0.2',
-    }
-    const result = await handler.buildAddLiquidityCallData({ inputs })
+    })
 
     expect(result.to).toBe(networkConfig.contracts.balancer.vaultV2)
     expect(result.data).toBeDefined()
   })
+
+  test('throws exception if we try to buildAddLiquidityCallData before the last queryAddLiquidity query has finished', async () => {
+    const humanAmountsIn: HumanAmountIn[] = [
+      { humanAmount: '1', tokenAddress: wETHAddress },
+      { humanAmount: '1', tokenAddress: wjAuraAddress },
+    ]
+
+    const handler = selectUnbalancedHandler()
+
+    // Store query response in handler instance
+    await handler.queryAddLiquidity(humanAmountsIn)
+
+    // Run without await so that the query is loading when we call buildAddLiquidityCallData
+    handler.queryAddLiquidity(humanAmountsIn)
+
+    const callback = async () =>
+      handler.buildAddLiquidityCallData({
+        account: defaultTestUserAccount,
+        slippagePercent: '0.2',
+      })
+
+    await expect(callback()).rejects.toThrowErrorMatchingInlineSnapshot(`
+      [SentryError: Missing queryResponse.
+      It looks that you tried to call useBuildCallData before the last query finished generating queryResponse]
+    `)
+  })
 })
 
 describe('When adding unbalanced liquidity for a stable pool', () => {
@@ -94,7 +116,7 @@ describe('When adding unbalanced liquidity for a stable pool', () => {
       }
     })
 
-    const priceImpact = await handler.calculatePriceImpact({ humanAmountsIn })
+    const priceImpact = await handler.calculatePriceImpact(humanAmountsIn)
     expect(priceImpact).toBeGreaterThan(0.001)
   })
 })

lib/modules/pool/actions/add-liquidity/handlers/UnbalancedAddLiquidity.handler.ts

@@ -1,3 +1,4 @@
+/* eslint-disable @typescript-eslint/no-non-null-assertion */
 import { getDefaultRpcUrl } from '@/lib/modules/web3/Web3Provider'
 import { TransactionConfig } from '@/lib/modules/web3/contracts/contract.types'
 import {
@@ -6,18 +7,15 @@ import {
   AddLiquidityQueryOutput,
   AddLiquidityUnbalancedInput,
   PriceImpact,
+  PriceImpactAmount,
   Slippage,
 } from '@balancer/sdk'
-import {
-  AddLiquidityInputs,
-  AddLiquidityOutputs,
-  BuildLiquidityInputs,
-  PriceImpactAmount,
-} from '../add-liquidity.types'
-import { AddLiquidityHandler } from './AddLiquidity.handler'
 import { Pool } from '../../../usePool'
 import { LiquidityActionHelpers, areEmptyAmounts } from '../../LiquidityActionHelpers'
 import { HumanAmountIn } from '../../liquidity-types'
+import { BuildAddLiquidityInputs, QueryAddLiquidityOutput } from '../add-liquidity.types'
+import { AddLiquidityHandler } from './AddLiquidity.handler'
+import { SentryError } from '@/lib/shared/utils/errors'
 
 /**
  * UnbalancedAddLiquidityHandler is a handler that implements the
@@ -28,24 +26,26 @@ import { HumanAmountIn } from '../../liquidity-types'
  */
 export class UnbalancedAddLiquidityHandler implements AddLiquidityHandler {
   helpers: LiquidityActionHelpers
-  sdkQueryOutput?: AddLiquidityQueryOutput
+  queryResponse?: AddLiquidityQueryOutput
 
   constructor(pool: Pool) {
     this.helpers = new LiquidityActionHelpers(pool)
   }
 
-  public async queryAddLiquidity({
-    humanAmountsIn,
-  }: AddLiquidityInputs): Promise<AddLiquidityOutputs> {
+  public async queryAddLiquidity(
+    humanAmountsIn: HumanAmountIn[]
+  ): Promise<QueryAddLiquidityOutput> {
+    // Deletes the previous queryResponse to enforce that we don't build callData with an outdated queryResponse (while a new one is loading)
+    this.queryResponse = undefined
     const addLiquidity = new AddLiquidity()
     const addLiquidityInput = this.constructSdkInput(humanAmountsIn)
 
-    this.sdkQueryOutput = await addLiquidity.query(addLiquidityInput, this.helpers.poolStateInput)
+    this.queryResponse = await addLiquidity.query(addLiquidityInput, this.helpers.poolStateInput)
 
-    return { bptOut: this.sdkQueryOutput.bptOut }
+    return { bptOut: this.queryResponse.bptOut }
   }
 
-  public async calculatePriceImpact({ humanAmountsIn }: AddLiquidityInputs): Promise<number> {
+  public async calculatePriceImpact(humanAmountsIn: HumanAmountIn[]): Promise<number> {
     if (areEmptyAmounts(humanAmountsIn)) {
       // Avoid price impact calculation when there are no amounts in
       return 0
@@ -64,23 +64,24 @@ export class UnbalancedAddLiquidityHandler implements AddLiquidityHandler {
   /*
     sdkQueryOutput is the result of the query that we run in the add liquidity form
   */
-  public async buildAddLiquidityCallData(
-    buildInputs: BuildLiquidityInputs
-  ): Promise<TransactionConfig> {
-    const { account, slippagePercent } = buildInputs.inputs
-    if (!account || !slippagePercent) throw new Error('Missing account or slippage')
-    if (!this.sdkQueryOutput) {
-      console.error('Missing sdkQueryOutput.')
-      throw new Error(
-        `Missing sdkQueryOutput.
-It looks that you did not call useAddLiquidityBtpOutQuery before trying to build the tx config`
+  public async buildAddLiquidityCallData({
+    account,
+    slippagePercent,
+  }: BuildAddLiquidityInputs): Promise<TransactionConfig> {
+    if (!this.queryResponse) {
+      // This should never happen because we don't allow the user to trigger buildAddLiquidityCallData
+      // before the query is loaded.
+      console.error('Missing queryResponse.')
+      throw new SentryError(
+        `Missing queryResponse.
+It looks that you tried to call useBuildCallData before the last query finished generating queryResponse`
       )
     }
 
     const addLiquidity = new AddLiquidity()
 
     const { call, to, value } = addLiquidity.buildCall({
-      ...this.sdkQueryOutput,
+      ...this.queryResponse,
       slippage: Slippage.fromPercentage(`${Number(slippagePercent)}`),
       sender: account,
       recipient: account,

lib/modules/pool/actions/add-liquidity/handlers/selectAddLiquidityHandler.ts

@@ -1,8 +1,8 @@
 import { getChainId } from '@/lib/config/app.config'
 import { Pool } from '../../../usePool'
-import { AddLiquidityHandler } from './AddLiquidity.handler'
 import { TwammAddLiquidityHandler } from './TwammAddLiquidity.handler'
 import { UnbalancedAddLiquidityHandler } from './UnbalancedAddLiquidity.handler'
+import { AddLiquidityHandler } from './AddLiquidity.handler'
 
 export function selectAddLiquidityHandler(pool: Pool) {
   // TODO: Depending on the pool attributes we will return a different handler

lib/modules/pool/actions/add-liquidity/queries/useAddLiquidityBuildCallDataQuery.ts

@@ -29,12 +29,7 @@ export function useAddLiquidityBuildCallDataQuery(
       humanAmountsIn,
     }),
     async () => {
-      const inputs = {
-        humanAmountsIn,
-        account: userAddress,
-        slippagePercent: slippage,
-      }
-      return handler.buildAddLiquidityCallData({ inputs })
+      return handler.buildAddLiquidityCallData({ account: userAddress, slippagePercent: slippage })
     },
     {
       enabled:

lib/modules/pool/actions/add-liquidity/queries/useAddLiquidityPreviewQuery.integration.spec.tsx

@@ -24,7 +24,7 @@ test('queries btp out for add liquidity', async () => {
 
   const result = await testQuery(humanAmountsIn)
 
-  await waitFor(() => expect(result.current.bptOut).not.toBeNull())
+  await waitFor(() => expect(result.current.bptOut).toBeDefined())
 
   expect(result.current.bptOut?.amount).toBeDefined()
   expect(result.current.isPreviewQueryLoading).toBeFalsy()