Skip to content

fix(token-swap): mint LP pro-rata, key first deposit on LP supply, enforce mint ordering - #10

Open
SwineCoder101 wants to merge 2 commits into
mainfrom
fix/token-swap-lp-accounting
Open

fix(token-swap): mint LP pro-rata, key first deposit on LP supply, enforce mint ordering#10
SwineCoder101 wants to merge 2 commits into
mainfrom
fix/token-swap-lp-accounting

Conversation

@SwineCoder101

Copy link
Copy Markdown
Owner

Bug: LP tokens are minted from sqrt(a * b) on every deposit, the first-deposit path is keyed on pool balances, and pool mints are not ordered

deposit_liquidity mints isqrt(amount_a * amount_b) LP tokens on every deposit regardless of the existing LP supply, so once swap fees have grown the reserves a later depositor is over-minted and can withdraw more than they put in, taking the fees earned by earlier LPs. The same instruction decides "first deposit" from pool_a.amount == 0 && pool_b.amount == 0, so anyone can transfer one base unit of a token straight to a pool token account before the first deposit and either brick the pool forever (division by zero panic on every deposit) or make the first depositor lose their tokens for 0 LP. Finally, create_pool does not enforce the mint_a < mint_b ordering the README documents, so two pools can exist for one pair. These are correctness/loss-of-funds bugs reachable by any unprivileged user.

Affected

  • anchor (the only variant)
  • tokens/token-swap/anchor/programs/token-swap/src/instructions/deposit_liquidity.rs
  • tokens/token-swap/anchor/programs/token-swap/src/instructions/create_pool.rs
  • tokens/token-swap/anchor/programs/token-swap/src/errors.rs
  • tokens/token-swap/anchor/Anchor.toml (test harness only, see Reproduce)
  • tokens/token-swap/anchor/tests/{utils,deposit-liquidity,create-pool}.ts

Functionality

create_pool creates the canonical constant-product pool for an (amm, mint_a, mint_b) triple. deposit_liquidity takes tokens in the pool's current ratio and mints LP tokens representing the depositor's share; withdraw_liquidity burns LP tokens and pays out amount * reserve / (supply + MINIMUM_LIQUIDITY) of each reserve. For that accounting to be fair, LP minted on a non-initial deposit must be proportional to the existing supply, and the first-deposit path must only run when no LP has been minted yet.

The bug

  1. Over-mint after fee accrual (deposit_liquidity.rs:68-80 on main): liquidity = isqrt(amount_a * amount_b) is used for every deposit; only the first deposit subtracts MINIMUM_LIQUIDITY. Swaps grow k = pool_a * pool_b through fees while LP supply is fixed, so sqrt(k) drifts above supply + MINIMUM_LIQUIDITY. A later depositor of (a, b) at the pool ratio gets sqrt(a * b) LP, which is more than the a * total / pool_a share they are entitled to. withdraw_liquidity.rs:25-29 then pays them lp * reserve / total, i.e. more than they deposited. In the test below (10M/10M seed, four 1M swaps at 5% fee, then a second deposit) the second depositor is minted 9,848,643 LP where the pro-rata amount is 9,755,701 and withdraws more of both tokens than they deposited.

  2. First-deposit detection keyed on reserves (deposit_liquidity.rs:34): pool_creation = pool_a.amount == 0 && pool_b.amount == 0. Anyone can spl-token transfer 1 unit directly into pool_account_b (a plain ATA) before the first deposit. Every deposit then takes the ratio path with pool_a = 0 and hits .checked_div(0).unwrap() at deposit_liquidity.rs:49-50, panicking. Tokens only leave the pool through withdraw_liquidity, which needs LP that can never be minted, and the pool PDA is deterministic per (amm, mint_a, mint_b), so the canonical pool is bricked. Donating 1 unit of A instead gives amount_b_required = amount_a * 0 / 1 = 0, so the depositor's A is transferred in and liquidity = isqrt(a * 0) = 0 LP is minted: the depositor loses their tokens.

  3. No mint ordering (create_pool.rs:31-42): README.md:191 says the pool account constraint "ensures mint_a's key is less than mint_b's key", supporting "precisely one pool" per pair (README.md:42). No such constraint exists, so (amm, A, B) and (amm, B, A) are both valid pools. The README states the intended behaviour, so the code was changed to match it rather than the other way round.

Reproduce

cd tokens/token-swap/anchor
pnpm install --frozen-lockfile
anchor build --ignore-keys   # anchor 1.0.2
anchor test --skip-build --validator legacy

Note: Anchor.toml had no [programs.localnet] entry, so anchor test loaded the .so at the ephemeral keypair address instead of declare_id!, and every test failed with "This program may not be used for executing instructions". The entry is added in this branch so the suite runs at all.

Tests added:

  • Create pool / Rejects mints out of order
  • Deposit liquidity / Second depositor cannot capture fees accrued by earlier depositors
  • Deposit liquidity / First deposit succeeds after token B is donated to the pool
  • Deposit liquidity / First deposit mints liquidity after token A is donated to the pool

Against the unmodified program:

  Create pool
    ✔ Creation (489ms)
    ✔ Invalid mints
    1) Rejects mints out of order
  Deposit liquidity
    ✔ Deposit equal amounts (479ms)
    ✔ Deposit with existing liquidity (same ratio) (926ms)
    ✔ Deposit with different ratio (939ms)
    2) Second depositor cannot capture fees accrued by earlier depositors
    3) First deposit succeeds after token B is donated to the pool
    4) First deposit mints liquidity after token A is donated to the pool
  ...
  10 passing (1m)
  4 failing

  1) Create pool
       Rejects mints out of order:
     Error: Expected transaction to fail with InvalidMint

  2) Deposit liquidity
       Second depositor cannot capture fees accrued by earlier depositors:
      AssertionError: expected '9848643' to equal '9755701'

  3) Deposit liquidity
       First deposit succeeds after token B is donated to the pool:
     Simulation failed.
Message: Transaction simulation failed: Error processing Instruction 0: Program failed to complete.
  "Program log: called `Option::unwrap()` on a `None` value",
  "Program UPxp2moQFWsGqfFd3ynqG2W9mj9CTjH68NN2bYUAqV1 failed: SBF program Panicked in programs/token-swap/src/instructions/deposit_liquidity.rs at 50:14"

  4) Deposit liquidity
       First deposit mints liquidity after token A is donated to the pool:
      AssertionError: expected '0' to equal '3999900'

Fix

deposit_liquidity.rs:

  • pool_creation is now mint_liquidity.supply == 0. Tokens donated to the pool accounts no longer change which path runs; they simply become part of the reserves shared by LPs.
  • The ratio path returns EmptyPoolReserves (new error) if either reserve is zero instead of panicking, and uses ok_or(MathOverflow)? instead of unwrap().
  • Non-initial deposits mint min(amount_a * total / pool_a, amount_b * total / pool_b) with total = supply + MINIMUM_LIQUIDITY, computed in u128 with checked ops and converted with u64::try_from. This is the same total that withdraw_liquidity divides by, so a deposit followed by a withdrawal returns at most what was deposited (rounding favours the pool) and fee growth stays with the LPs who earned it. A deposit that rounds to 0 LP is rejected with DepositTooSmall instead of taking the tokens for nothing.
  • The first deposit keeps isqrt(a * b) - MINIMUM_LIQUIDITY.

create_pool.rs: constraint = mint_a.key() < mint_b.key() @ TutorialError::InvalidMint on the pool account, matching the README. tests/utils.ts already sorted the generated mints; createValues now also honours explicitly supplied mints so the test can build the reversed pair.

Verification

cd tokens/token-swap/anchor
anchor build --ignore-keys
anchor test --skip-build --validator legacy
  Create AMM
    ✔ Creation (457ms)
    ✔ Invalid fee
  Create pool
    ✔ Creation (477ms)
    ✔ Invalid mints
    ✔ Rejects mints out of order
  Deposit liquidity
    ✔ Deposit equal amounts (482ms)
    ✔ Deposit with existing liquidity (same ratio) (1369ms)
    ✔ Deposit with different ratio (1000ms)
    ✔ Second depositor cannot capture fees accrued by earlier depositors (6051ms)
    ✔ First deposit succeeds after token B is donated to the pool (976ms)
    ✔ First deposit mints liquidity after token A is donated to the pool (2611ms)
  Swap
    ✔ Swap from A to B (490ms)
    ✔ Swap from B to A (492ms)
  Withdraw liquidity
    ✔ Withdraw everything (491ms)

  14 passing (1m)

cargo fmt applied to the changed files; cargo clippy -p swap_example -- -D warnings -A deprecated is clean (the crate is in .github/.workspace-ignore and has pre-existing AccountInfo deprecation lints in files this change does not touch). pnpm exec tsc --noEmit passes.

… bricking via donation, and unordered pool mints

- Second depositor after swaps receives sqrt(a*b) LP instead of a pro-rata
  share and can withdraw more than deposited.
- Transferring 1 unit of B to pool_account_b before the first deposit makes
  every deposit panic on a division by zero; donating A instead mints 0 LP.
- create_pool accepts mint_a > mint_b despite the documented ordering
  constraint, allowing two pools per pair.
- Add [programs.localnet] so anchor test loads the program at declare_id.
…force mint ordering

- Non-initial deposits now mint min(a * total / pool_a, b * total / pool_b)
  with total = supply + MINIMUM_LIQUIDITY (the figure withdraw_liquidity
  divides by), so fees accrued to the reserves stay with existing LPs.
- The first-deposit path is selected by mint_liquidity.supply == 0 instead of
  empty reserves, so tokens transferred directly to the pool accounts can no
  longer brick the pool or mint 0 LP; a zero reserve on the ratio path returns
  EmptyPoolReserves instead of panicking.
- create_pool requires mint_a < mint_b as documented in the README.
@SwineCoder101
SwineCoder101 force-pushed the fix/token-swap-lp-accounting branch from f122d25 to 61e7e73 Compare August 27, 2026 12:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant