Skip to content

fix add liquidity#11

Merged
gabririgo merged 10 commits into
mainfrom
fix/add-liquidity
May 25, 2026
Merged

fix add liquidity#11
gabririgo merged 10 commits into
mainfrom
fix/add-liquidity

Conversation

@gabririgo

@gabririgo gabririgo commented May 25, 2026

Copy link
Copy Markdown
Contributor
  • correctly add liquidity
  • prompt user to initialize pool from user's wallet if pool has not been initialized (smart pools can only add liquidity to already-initialized liquidity pools)

@gabririgo gabririgo requested a review from Copilot May 25, 2026 16:13
Copilot AI reviewed May 25, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the Uniswap v4 LP flow so liquidity adds fail fast on uninitialized pools and introduces a new initialize_pool tool to build a PoolManager initialize transaction (signed from the user wallet) before adding liquidity via the vault.

Changes:

  • Add initialize_pool as a first-class tool (LLM definition, prompts, routing/categorying, dispatch) and include it in required-tool tests.
  • Update add_liquidity to explicitly block pools with sqrtPriceX96 = 0 and improve error guidance around initialization.
  • Extend get_pool_info messaging to suggest initialization when appropriate.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
tests/tools.test.ts Adds initialize_pool to the required tool list to prevent accidental removal.
src/services/uniswapLP.ts Adds buildInitializePoolTx, exports POOL_MANAGER, and changes pool-state/“initialized” handling for add-liquidity + pool info.
src/routes/tools.ts Categorizes initialize_pool under “Uniswap v4 LP”.
src/llm/tools.ts Defines the new initialize_pool tool and updates LP tool guidance around tickSpacing/initialization.
src/llm/prompts.ts Updates LP workflow instructions to include initialization.
src/llm/client.ts Wires the new tool into execution and updates get_pool_info output messaging.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/services/uniswapLP.ts Outdated
Comment thread src/services/uniswapLP.ts
Comment thread src/services/uniswapLP.ts Outdated
Comment thread src/services/uniswapLP.ts
Comment thread src/llm/client.ts Outdated
Comment thread src/services/uniswapLP.ts
@gabririgo
fix: address Copilot review comments on initialize_pool
115b3c5 
- Replace floating-point sqrtPriceX96 math with pure BigInt integer
  arithmetic (Newton-Raphson isqrt) to avoid precision loss and
  Infinity/overflow for large ERC-20 base-unit amounts
- Add input validation: throw when amount0 or amount1 is zero
- Add TickMath bounds check on computed sqrtPriceX96
- Export computeSqrtPriceX96FromAmounts for unit testing
- Add @uniswap/v3-sdk and jsbi as direct package.json dependencies
  (previously only transitive via @uniswap/v4-sdk)
- Fix inaccurate catch comment in buildInitializePoolTx: readPoolState
  returns sqrtPriceX96=0 for uninitialized pools (does not throw);
  the catch block handles RPC/StateView deployment failures
- Fix getPoolInfoById to return structured PoolInfo with initialized=false
  instead of throwing when pool is uninitialized, enabling get_pool_info
  to surface the initialization guidance message in client.ts
- Add unit tests for computeSqrtPriceX96FromAmounts covering precision,
  edge cases, zero inputs, out-of-bounds ratios, and symmetry

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@gabririgo gabririgo requested a review from Copilot May 25, 2026 16:24
Copilot AI reviewed May 25, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.

Comment thread src/services/uniswapLP.ts
Comment thread src/llm/client.ts Outdated
Comment thread src/services/uniswapLP.ts
Comment thread tests/uniswapLP.test.ts Outdated
@gabririgo
fix: address round-2 Copilot review comments
4bebd7f 
- Distinguish infrastructure errors (StateView not deployed, RPC failures)
  from pool-key mismatches in the readPoolState catch block so users are
  not misled into retrying with a different pool key when the backend simply
  cannot read on-chain state
- Add poolKeyKnown: boolean to PoolInfo interface; set false for uninitialized
  pools (where fee/tickSpacing/hooks/currency0/currency1 are zero placeholders)
  and true when values come from the on-chain Initialize event
- Update get_pool_info message in client.ts: when poolKeyKnown is false, ask
  the user for the full pool key instead of presenting placeholder values as
  valid initialize_pool parameters
- Tighten symmetry test tolerance from Q96 (~7.9e28) to 2n, matching the
  comment intent of allowing ±1 floor rounding

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@gabririgo gabririgo requested a review from Copilot May 25, 2026 16:32
Copilot AI reviewed May 25, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Comment thread src/services/uniswapLP.ts
@gabririgo
fix: validate user-provided sqrtPriceX96 in buildInitializePoolTx
20e179c 
When params.sqrtPriceX96 is supplied directly, add explicit bounds
validation (>0, within [MIN_SQRT_RATIO, MAX_SQRT_RATIO)) before passing
the value to TickMath/encodeFunctionData, consistent with the checks
already present in computeSqrtPriceX96FromAmounts. Throws a user-friendly
error rather than a low-level revert when the value is out of range.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@gabririgo gabririgo requested a review from Copilot May 25, 2026 16:38
Copilot AI reviewed May 25, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Comment thread src/llm/tools.ts
@gabririgo
fix: add oneOf constraint to initialize_pool tool schema
4dbef72 
Encode the mutual-exclusion between sqrtPriceX96 and (amountA+amountB)
directly in the JSON Schema parameters object. The top-level required
array continues to mandate tokenA, tokenB, and fee; a oneOf clause adds
that callers must also supply either sqrtPriceX96 OR both amountA AND
amountB. This prevents the LLM from invoking the tool with an argument
combination that would cause a runtime error in buildInitializePoolTx.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@gabririgo gabririgo requested a review from Copilot May 25, 2026 16:45
Copilot AI reviewed May 25, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Comment thread src/services/uniswapLP.ts Outdated
Comment thread src/llm/tools.ts
@gabririgo
fix: restore remove_liquidity description and use hasInitializeEvent …
6479030 
…in initialized check

Two fixes:

1. tools.ts: restore the leading sentence of remove_liquidity's description
   ('Remove liquidity from a Uniswap v4 LP position through the vault's
   modifyLiquidities adapter.') that was accidentally dropped when the
   oneOf clause was added to the adjacent initialize_pool tool in the
   previous commit.

2. uniswapLP.ts: in getPoolInfoById, treat hasInitializeEvent as
   authoritative for the initialized flag. Changed:
     initialized: sqrtPriceX96 !== 0n
   to:
     initialized: hasInitializeEvent || sqrtPriceX96 !== 0n
   An on-chain Initialize event is definitive proof the pool exists.
   If sqrtPriceX96 reads back as 0 due to a transient RPC/StateView
   issue, the pool should not be incorrectly classified as uninitialized
   (which would generate misleading 'call initialize_pool' guidance that
   would result in an on-chain revert).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@gabririgo gabririgo requested a review from Copilot May 25, 2026 16:52
Copilot AI reviewed May 25, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Comment thread src/services/uniswapLP.ts
Comment thread src/services/uniswapLP.ts Outdated
@gabririgo
fix: improve JSDoc and catch block in buildInitializePoolTx
b7950b3 
Two fixes in uniswapLP.ts:

1. JSDoc for computeSqrtPriceX96FromAmounts: add a second @throws
   entry documenting that the function also throws when the computed
   sqrtPriceX96 falls outside TickMath bounds (extreme price ratios).
   Previously only the zero-amount throw was documented.

2. buildInitializePoolTx catch block: only swallow StateView-unavailable
   errors (expected on chains without StateView deployment). Any other
   error — e.g. transient RPC failures — is now re-thrown as a user-
   friendly 'Could not verify pool initialization state' message. This
   prevents silently proceeding to build a transaction that would
   revert when the pre-initialization check failed for a non-structural
   reason.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@gabririgo gabririgo requested a review from Copilot May 25, 2026 17:00
Copilot AI reviewed May 25, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Comment thread src/services/uniswapLP.ts Outdated
Comment thread src/services/uniswapLP.ts Outdated
Comment thread package.json
@gabririgo
fix: poolKeyKnown flag, sqrtPriceX96=0 error message, update lockfiles
f9b9891 
- Set poolKeyKnown: hasInitializeEvent (not unconditionally true)
  so callers know when pool key fields come from the on-chain Initialize
  event vs. are placeholder/estimated values
- Improve sqrtPriceX96=0 error in buildAddLiquidityTx to reflect both
  possible causes: pool uninitialized OR wrong pool key
  (fee/tickSpacing/hooks mismatch), and suggest get_pool_info to verify
- Update package-lock.json and yarn.lock to include @uniswap/v3-sdk
  and jsbi as tracked direct runtime dependencies

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@gabririgo gabririgo requested a review from Copilot May 25, 2026 17:07
Copilot AI reviewed May 25, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 8 out of 10 changed files in this pull request and generated 2 comments.

Comment thread src/llm/client.ts Outdated
Comment thread src/services/uniswapLP.ts
@gabririgo
fix: poolKeyKnown warning in get_pool_info, same-token guard in build…
070b8ac 
…InitializePoolTx

  that warns pool key fields may be placeholders (Initialize event not found)
  and asks user to verify before calling add_liquidity, instead of silently
  suggesting potentially wrong fee/tickSpacing/hooks values
- In buildInitializePoolTx: add explicit check that tokenA and tokenB resolve
  to different addresses after resolution, with a clear error message

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@gabririgo gabririgo requested a review from Copilot May 25, 2026 17:15
Copilot AI reviewed May 25, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 8 out of 10 changed files in this pull request and generated 1 comment.

Comment thread src/services/uniswapLP.ts
@gabririgo
fix: validate fee and tickSpacing ABI ranges in pool key operations
199c1bc 
Add validatePoolKeyNumerics() helper that checks fee (uint24: 0-16,777,215,
integer) and tickSpacing (int24: >0 and ≤8,388,607, integer) before building
pool key / pool ID. Called in both buildAddLiquidityTx and buildInitializePoolTx
to catch invalid inputs with a clear message instead of low-level ABI encoding
errors or on-chain reverts.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@gabririgo gabririgo requested a review from Copilot May 25, 2026 17:23
Copilot AI reviewed May 25, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 8 out of 10 changed files in this pull request and generated no new comments.

@gabririgo gabririgo merged commit abbf7c2 into main May 25, 2026
1 check passed
@gabririgo gabririgo deleted the fix/add-liquidity branch May 25, 2026 18:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@gabririgo