feat(token-swap): add pinocchio example - #716
Conversation
Greptile SummaryThe PR adds a Pinocchio implementation of the token-swap example, including AMM and pool creation, liquidity management, swaps, tests, and workspace integration. The latest account-binding changes rederive the pool authority, canonical reserve accounts, and liquidity mint before those accounts participate in settlement.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains; the current canonical reserve and liquidity-mint checks address all three previously reported account-substitution paths before pricing, share accounting, or settlement. Important Files Changed
Reviews (2): Last reviewed commit: "token-swap: bind the pool vaults and liq..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| }; | ||
| let input = input_amount.min(held); |
There was a problem hiding this comment.
When a trader supplies a zero-balance counterfeit paying-side reserve together with the genuine opposite reserve, pricing returns the genuine reserve's balance and the zero-valued invariant check still passes, allowing that vault to be drained. Bind both reserve accounts to the pool authority's canonical token accounts for the stored mints. How this was verified: The reserve arguments are omitted from PoolSeeds::load but are used directly for pricing, PDA-signed settlement, and both invariant calculations.
Knowledge Base Used:
There was a problem hiding this comment.
All three are correct and share one root cause — fixed in 3b767698.
PoolSeeds::load rederived the pool and its authority but never checked the two vaults, and neither deposit_liquidity nor withdraw_liquidity checked the liquidity mint. Anchor gets exactly those from associated_token::mint / associated_token::authority and the mint's seeds constraint; I dropped them when porting to explicit checks, and each omission is independently drainable:
- swap — a zero-balance stand-in for the paying reserve prices the trade against nothing, so the genuine opposite vault pays out its whole balance and the invariant check passes against a zero product;
- deposit — reserves the caller controls receive the transfers while the pool still PDA-mints genuine LP shares, redeemable against the real reserves;
- withdraw — a caller-created LP mint sets the entitlement denominator, so they name their own share.
Both vaults are now rederived as the pool authority's associated token accounts, and the liquidity mint from [amm, mint_a, mint_b, b"liquidity"].
Verified rather than assumed: three new tests, one per vector. The swap and deposit ones build a token account that is owned by the pool authority and does hold the right mint but is not the ATA — so a mint-and-owner check would not have caught them, only the address derivation does. Against the previous build all three transactions succeed; with the fix each is rejected with InvalidSeeds and the reserves are asserted untouched. 13 tests passing.
|
|
||
| let supply = mint_supply(&mint_liquidity.try_borrow()?)?; | ||
| let denominator = supply.checked_add(MINIMUM_LIQUIDITY).ok_or(SwapError::MathOverflow)?; | ||
|
|
||
| let amount_a = mul_div(amount, token_amount(&pool_account_a.try_borrow()?)?, denominator)?; |
There was a problem hiding this comment.
When an attacker supplies a self-created liquidity mint while passing the genuine pool vaults, that mint's supply determines the withdrawal entitlement and its counterfeit tokens are burned after the pool PDA releases genuine assets, allowing nearly all reserves to be withdrawn without genuine LP shares. How this was verified: The handler never derives mint_liquidity using LIQUIDITY_SEED, yet reads its supply for entitlement and burns from it after both PDA-signed vault transfers.
Knowledge Base Used: Token escrow, swaps, and fundraising
There was a problem hiding this comment.
All three are correct and share one root cause — fixed in 3b767698.
PoolSeeds::load rederived the pool and its authority but never checked the two vaults, and neither deposit_liquidity nor withdraw_liquidity checked the liquidity mint. Anchor gets exactly those from associated_token::mint / associated_token::authority and the mint's seeds constraint; I dropped them when porting to explicit checks, and each omission is independently drainable:
- swap — a zero-balance stand-in for the paying reserve prices the trade against nothing, so the genuine opposite vault pays out its whole balance and the invariant check passes against a zero product;
- deposit — reserves the caller controls receive the transfers while the pool still PDA-mints genuine LP shares, redeemable against the real reserves;
- withdraw — a caller-created LP mint sets the entitlement denominator, so they name their own share.
Both vaults are now rederived as the pool authority's associated token accounts, and the liquidity mint from [amm, mint_a, mint_b, b"liquidity"].
Verified rather than assumed: three new tests, one per vector. The swap and deposit ones build a token account that is owned by the pool authority and does hold the right mint but is not the ATA — so a mint-and-owner check would not have caught them, only the address derivation does. Against the previous build all three transactions succeed; with the fix each is rejected with InvalidSeeds and the reserves are asserted untouched. 13 tests passing.
| let seeds = PoolSeeds::load(program_id, pool, pool_authority, mint_a, mint_b)?; | ||
|
|
||
| // Never take more than the depositor holds. | ||
| let mut amount_a = requested_a.min(token_amount(&depositor_account_a.try_borrow()?)?); | ||
| let mut amount_b = requested_b.min(token_amount(&depositor_account_b.try_borrow()?)?); | ||
|
|
||
| let pool_a = token_amount(&pool_account_a.try_borrow()?)?; | ||
| let pool_b = token_amount(&pool_account_b.try_borrow()?)?; |
There was a problem hiding this comment.
When a depositor supplies token accounts they control as the pool reserves while retaining the genuine liquidity mint, the transfers return the assets to attacker-controlled accounts but the pool PDA still mints genuine LP shares, which can then redeem assets from the real reserves. How this was verified: PoolSeeds::load omits both reserve accounts, while their balances drive share issuance and the transfers target them immediately before genuine PDA-authorized LP minting.
Knowledge Base Used:
There was a problem hiding this comment.
All three are correct and share one root cause — fixed in 3b767698.
PoolSeeds::load rederived the pool and its authority but never checked the two vaults, and neither deposit_liquidity nor withdraw_liquidity checked the liquidity mint. Anchor gets exactly those from associated_token::mint / associated_token::authority and the mint's seeds constraint; I dropped them when porting to explicit checks, and each omission is independently drainable:
- swap — a zero-balance stand-in for the paying reserve prices the trade against nothing, so the genuine opposite vault pays out its whole balance and the invariant check passes against a zero product;
- deposit — reserves the caller controls receive the transfers while the pool still PDA-mints genuine LP shares, redeemable against the real reserves;
- withdraw — a caller-created LP mint sets the entitlement denominator, so they name their own share.
Both vaults are now rederived as the pool authority's associated token accounts, and the liquidity mint from [amm, mint_a, mint_b, b"liquidity"].
Verified rather than assumed: three new tests, one per vector. The swap and deposit ones build a token account that is owned by the pool authority and does hold the right mint but is not the ATA — so a mint-and-owner check would not have caught them, only the address derivation does. Against the previous build all three transactions succeed; with the fix each is rejected with InvalidSeeds and the reserves are asserted untouched. 13 tests passing.
PoolSeeds::load rederived the pool and its authority but never checked
the two vaults, and neither deposit nor withdraw checked the liquidity
mint. Anchor gets those from associated_token::mint/authority and the
mint's seeds constraint; dropping them in the port left three ways to
drain a pool:
- swap with a zero-balance stand-in for the paying reserve prices the
trade against nothing and empties the opposite vault;
- deposit into caller-controlled reserves still mints genuine LP
shares, redeemable against the real ones;
- withdraw against a caller-created LP mint sets the entitlement to
whatever they like.
Rederive both vaults as the authority's associated token accounts, and
the liquidity mint from its seeds. Each vector has a test verified to
succeed without the checks.
1 participant
Adds a Pinocchio implementation of
token-swap, alongside the existing Anchor one.What it does
A constant-product AMM. Five instructions:
CreateAmm,CreatePool,DepositLiquidity,WithdrawLiquidityandSwapExactTokensForTokens.Everything a pool owns hangs off a single authority PDA — both vaults and the LP mint — so the program can move pool funds without any wallet holding that power.
The parts worth reading
MINIMUM_LIQUIDITYis burned on the first deposit and never minted to anyone. It keeps the pool from being emptied completely, which is what would otherwise let the share price be skewed while the pool is near-empty. Withdrawals divide bysupply + MINIMUM_LIQUIDITYfor the same reason.a * bis read again from the vaults after the transfers; a higher value is fine (rounding in the pool's favour), a lower one aborts.amountfails at the burn and rolls the two transfers back, so the pool cannot be drained by asking for more than you hold.All the arithmetic goes through a
mul_divhelper that widens tou128, so the product of twou64balances cannot overflow.Account binding
The pool records its AMM and both mints.
PoolSeeds::loadreads them back, checks the supplied mints against the stored ones, and rederives both the pool and the authority — so a caller cannot pair a real pool with unrelated token accounts, or point a pool at a cheaper AMM's fee. There is a test for the mint substitution.Differences from the Anchor version
swap_ais au8.CreateAccount, so a stray lamport on a derivable address cannot block pool creation (see feat(merkle-tree-token-claimer): add pinocchio example #714).Tests
10 LiteSVM tests: the full lifecycle from AMM through pool, deposit, swap, ratio-trimmed deposit and withdrawal, plus fee, slippage and mint-substitution rejections. The swap test recomputes the expected output from the curve in the test rather than asserting a hardcoded number. Verified locally:
tsc --noEmit,pnpm test,prettier --check,cargo fmt --check,cargo clippy -D warnings.