feat(slippage): SSOT 7.3 fix — 4 DEX adapters + CLI + 17 regression tests by Hiksang · Pull Request #3 · hypurrquant/defi-cli

Hiksang · 2026-05-05T08:43:10Z

Summary

F1 follow-up tracked in PR #2
(qa/2026-05-05-test-foundation report). Wires real slippage protection into
all 4 DEX adapters that the SSOT 7.3 audit flagged.

Coverage: 12 of the 15 KNOWN_INFINITE_SLIPPAGE sites are broadcast
paths and are fixed here. The remaining 3 sites live inside quote()
internal simulation calls (used purely to extract amountOut from a
static call return) and are intentionally left at 0n — they never
broadcast.

Changes by adapter

Adapter	Sites fixed	Strategy
`uniswap_v3.ts`	4 (swap, mint × 2 variants, remove)	swap auto-quotes when no override; mint defaults to 50 bps; remove requires explicit `amount_{a,b}_min`
`thena_cl.ts`	3 (swap, mint, remove)	swap requires `amount_out_min` (no quoter); mint defaults to 50 bps; remove requires explicit min
`algebra_v3.ts`	4 (swap, mint × 2 ABI variants, remove)	swap auto-quotes via the algebra quoter; mint defaults to 50 bps; remove requires explicit min
`balancer_v3.ts`	1 (swap)	swap requires `amount_out_min` (no quoter wired)

For all mint paths, per-side amount_{a,b}_min overrides are accepted
and sorted onto the V3 token0/token1 axis based on token_a vs
token_b lexical order.

Types changes (commit `475e5e2`)

packages/defi-core/src/types.ts adds optional fields, all backward-compatible:

SwapParams.amount_out_min?: bigint
AddLiquidityParams.slippage?: Slippage, amount_a_min?, amount_b_min?
RemoveLiquidityParams.slippage?: Slippage, amount_a_min?, amount_b_min?

CLI changes (commit `0d86ecd`)

commands/lp.ts exposes new flags on lp add, lp farm, lp remove:

--slippage <bps> — basis-points tolerance (default 50 = 0.5%).
--amount-a-min <wei> / --amount-b-min <wei> — explicit per-side floor (overrides --slippage).

For V3-style lp remove, the per-side minimums are required — see the F1 commits for the rationale (no on-chain quoter for the remove path).

Tests added (commits `bca992b` + `31d069c`)

17 new regression tests across 4 adapter test files:

uniswap_v3.test.ts (7) — swap override; mint default 50 bps; mint override in both token orderings; remove rejects without minimums; remove sorts in both orderings.
thena_cl.test.ts (4) — swap rejects without override; swap forwards override; remove rejects without minimums; remove sorts.
algebra_v3.test.ts (4) — swap with override; mint default 50 bps (NEST-style v2 path via pool_deployer flag); remove rejects; remove sorts.
balancer_v3.test.ts (2) — swap rejects without override; swap forwards override.

Total monorepo test count: 78 → 95 (+22%).

Test plan

Host: pnpm -C ts -r build clean.
Host: pnpm -C ts -r test → 95/95 pass (defi-core 28 + defi-protocols 38 + defi-cli 29).
Host: pnpm -C ts -r lint clean.
Docker: requires PR QA: SSOT establishment + test foundation (v1.0.12 baseline) #2 (Dockerfile) to merge first.
Reviewer to validate: token0/token1 sort branch on a real V3 mainnet position.
Reviewer to validate: caller-required amount_{a,b}_min on lp remove is acceptable as a breaking-change-style hardening (alternative: compute from positions(tokenId) + pool.slot0 inside the adapter — heavier change, can ship as a follow-up).

Coordination with PR #2

PR #2 (qa/2026-05-05-v1-0-12-baseline) carries the snapshot test that
locks KNOWN_INFINITE_SLIPPAGE. Merge order recommendation:

PR QA: SSOT establishment + test foundation (v1.0.12 baseline) #2 first (docs/test/Dockerfile only — zero code-change risk).
This PR (feat/slippage-protection).
PR QA: SSOT establishment + test foundation (v1.0.12 baseline) #2 follow-up commit emptying KNOWN_INFINITE_SLIPPAGE — flips the gate from "snapshot freeze" to "any 0n minimum is a hard error".

Commit history

31d069c test(slippage): regression tests for thena_cl/algebra_v3/balancer_v3
0d86ecd feat(cli): expose --slippage / --amount-{a,b}-min on lp add/farm/remove
8523690 feat(balancer_v3): wire slippage protection into buildSwap
7d05ca8 feat(algebra_v3): wire slippage protection into 4 broadcast sites
0671501 feat(thena_cl): wire slippage protection into 3 broadcast sites
bca992b feat(uniswap_v3): wire slippage protection into 4 broadcast sites
475e5e2 refactor(types): add slippage + min-amount overrides to LP params

🤖 Generated with Claude Code

Extends SwapParams / AddLiquidityParams / RemoveLiquidityParams with the slippage knobs that the SSOT 7.3 fix needs. All additions are optional and backward-compatible (defaults to defaultSwapSlippage() inside adapters): - SwapParams.amount_out_min (override; otherwise quote-derived) - AddLiquidityParams.slippage (per-side floor source) - AddLiquidityParams.amount_a_min, amount_b_min (explicit override) - RemoveLiquidityParams.slippage, amount_a_min, amount_b_min (same) Adapters in defi-protocols still hard-code 0n today; this commit only prepares the parameter surface so the per-adapter fixes that follow have a typed contract to honor. No runtime behaviour change yet — build clean across 3 packages.

SSOT Section 7.3 fix for the UniswapV3Adapter — first of four DEX adapters covered by F1 (KNOWN_INFINITE_SLIPPAGE snapshot in the qa/2026-05-05-baseline branch). Changes: 1. buildSwap (was: amountOutMinimum = 0n hard-coded). - Honors params.amount_out_min override verbatim when provided. - Otherwise calls this.quote() and applies applyMinSlippage( params.slippage, quoted.amount_out) -- never falls back to 0n. - The auto-quote path requires an RPC; broadcast-time failures are loud rather than silent. 2. buildAddLiquidity (mint, both standard + slipstream variants). - Defaults to applyMinSlippage(slippage ?? 50bps, amountDesired) per side. - Honors per-side params.amount_a_min / amount_b_min overrides. - Sorts overrides onto the V3 token0/token1 axis based on params.token_a vs token_b lex order. 3. buildRemoveLiquidity (decreaseLiquidity). - Caller MUST now pass amount_a_min and amount_b_min. Computing them from on-chain state requires sqrtPriceMath + tick math; we leave that to lp.ts (next commit) and refuse to ship a 0n floor. - When provided, overrides are sorted onto amount0Min/amount1Min by the same token_a-vs-token_b rule used in buildAddLiquidity. The L262 amountOutMinimum: 0n inside quote() is intentional — it builds a swap-call simulation purely to extract amountOut from the return value, no real broadcast — and is left untouched. Tests (ts/packages/defi-protocols/src/dex/uniswap_v3.test.ts): - buildSwap honors amount_out_min override. - buildAddLiquidity defaults to 50 bps slippage on both axes. - buildAddLiquidity honors amount_{a,b}_min in both token_a < / > token_b orderings (sort branch coverage). - buildRemoveLiquidity rejects calls missing amount_{a,b}_min. - buildRemoveLiquidity sorts amount_{a,b}_min onto amount{0,1}Min in both token orderings. Verified: monorepo build/test/lint all clean — 85/85 tests pass (defi-core 28 + defi-protocols 28 + defi-cli 29). Follow-ups in this PR: - thena_cl, algebra_v3, balancer_v3 adapters (3 sites + 4 + 1). - lp.ts CLI plumbing for --slippage flag and per-side min overrides. Refs: F1 in docs/qa-reports/2026-05-05-test-foundation.md.

SSOT 7.3 fix for ThenaCLAdapter. Thena CL has no on-chain quoter (quote() throws unsupported), so the swap path requires the caller to pin amount_out_min explicitly — the adapter refuses to ship 0n. Changes: 1. buildSwap (was: amountOutMinimum = 0n) - Throws when params.amount_out_min is undefined. - Forwards the override verbatim into ExactInputSingleParams. 2. buildAddLiquidity mint (was: amount0Min = 0n, amount1Min = 0n) - Defaults to applyMinSlippage(slippage ?? 50bps, amountDesired) per side; honors per-side amount_{a,b}_min override. - Sorts overrides onto the V3 token0/token1 axis using the same token_a vs token_b lex rule as uniswap_v3. 3. buildRemoveLiquidity decreaseLiquidity (was: amount0Min/1Min = 0n) - Throws unless caller passes amount_a_min and amount_b_min. - Sorts overrides onto amount{0,1}Min by token_a/token_b lex. Verified: build + lint clean across 3 packages. Adapter-level unit tests for thena_cl come in the regression-tests commit at the end of this PR. Refs: F1 in docs/qa-reports/2026-05-05-test-foundation.md.

SSOT 7.3 fix for AlgebraV3Adapter (NEST-style + Algebra Integral forks like KittenSwap). Algebra exposes a quoter, so buildSwap can auto-derive amountOutMinimum the same way uniswap_v3 does. Changes: 1. buildSwap (was: amountOutMinimum = 0n) - Honors params.amount_out_min override. - Otherwise calls this.quote() and applies applyMinSlippage( params.slippage, quoted.amount_out). Quote failures (no quoter, no rpcUrl) bubble up rather than silently broadcasting 0n. 2. buildAddLiquidity — both branches: - algebraV2PmAbi (NEST-style, no `deployer` field) - algebraIntegralPmAbi (KittenSwap-style, has `deployer`) Defaults to applyMinSlippage(slippage ?? 50bps, amountDesired) per side. Honors per-side amount_{a,b}_min override sorted onto token0/token1 by token_a vs token_b lex order. 3. buildRemoveLiquidity decreaseLiquidity (was: amount0/1Min = 0n) - Throws unless caller passes amount_a_min and amount_b_min. - Sorts overrides onto amount{0,1}Min via the lex rule. Verified: build + lint clean across 3 packages. Refs: F1 in docs/qa-reports/2026-05-05-test-foundation.md.

SSOT 7.3 fix for BalancerV3Adapter — single broadcast site. The adapter's quote() is a stub (`throw DefiError.unsupported`), so the swap path requires the caller to pin amount_out_min explicitly. The adapter no longer ships minAmountOut = 0n. The pool-resolution path is still a TODO (zeroAddress placeholder), so the swap will fail at broadcast for unrelated reasons today; this commit ensures that once the pool wiring lands, slippage protection is already enforced. buildAddLiquidity / buildRemoveLiquidity are pool-specific stubs and do not have a 0n broadcast site to fix. Verified: build + lint clean across 3 packages. Refs: F1 in docs/qa-reports/2026-05-05-test-foundation.md.

Wires the slippage knobs introduced in defi-protocols through the lp.ts CLI so end users can opt into the new SSOT 7.3 protections without dropping into the adapter API directly. New options on `lp add`, `lp farm`, `lp remove`: --slippage <bps> Slippage tolerance in basis points (default 50 = 0.5%). Sets amount{0,1}Min per side via applyMinSlippage. --amount-a-min <wei> Explicit floor for token_a (overrides --slippage on that side). --amount-b-min <wei> Explicit floor for token_b. For `lp remove` against V3 / Algebra / Thena CL adapters, the adapter now refuses to ship without amount_{a,b}_min — see the F1 commits in this PR for the rationale (no quoter, can't safely auto-derive). The CLI option help text flags this requirement explicitly. `lp compound` already had --slippage from before; that stays as is. Verified: build + lint clean across 3 packages.

Pin the SSOT 7.3 invariants for the three remaining adapters in the F1 fix. Mirrors the pattern from uniswap_v3.test.ts: - thena_cl.test.ts (4 tests) - buildSwap rejects without amount_out_min (no quoter wired). - buildSwap forwards amount_out_min override verbatim. - buildRemoveLiquidity rejects without amount_{a,b}_min. - buildRemoveLiquidity sorts overrides onto amount{0,1}Min. - balancer_v3.test.ts (2 tests) - buildSwap rejects without amount_out_min. - buildSwap forwards amount_out_min override verbatim. - algebra_v3.test.ts (4 tests) - buildSwap honors amount_out_min override (skips quote path). - buildAddLiquidity defaults to 50 bps slippage on both axes (uses NEST-style v2 mint via pool_deployer flag). - buildRemoveLiquidity rejects without amount_{a,b}_min. - buildRemoveLiquidity sorts overrides onto amount{0,1}Min. Total monorepo test count: 78 -> 95 (+17 across the F1 PR — uniswap_v3 contributed 7 in an earlier commit; this one adds 10 more). Verified: 95/95 tests pass, lint clean across 3 packages.

…rement (#5) P2 follow-up tracked in qa/2026-05-05-test-foundation. Establishes a quantitative coverage baseline so the next QA cycle can pick targets data-driven instead of guessing. Tooling changes: - ts/package.json: add @vitest/coverage-v8 ^3.0.0 (vitest official dev dep; npmjs.com/package/@vitest/coverage-v8). User-approved per SSOT Section 3. - ts/package.json: add `test:coverage` script that delegates to each workspace's own vitest run. Single root invocation reads 0% line coverage because the workspace dist alias intercepts source imports; per-package execution sees the actual src files. - ts/vitest.config.ts: workspace-package src aliases (used by future cross-package tests; root coverage path is preferred per-package). - packages/{defi-core,defi-protocols,defi-cli}/vitest.config.ts: per-package coverage block (provider=v8, text-summary reporter). - .gitignore: ts/coverage/ + ts/packages/*/coverage/. Baseline measurement (main @ ae5bb65): | package | lines | covered | % | |-----------------|------:|--------:|-------:| | defi-core | 399 | 261 | 65.4% | | defi-protocols | 5976 | 5880 | 98.4% | | defi-cli | 6299 | 961 | 15.3% | | **monorepo** | 12674 | 7102 | 56.0% | Headlines: 1. defi-cli at 15.3% — 5338 of 6299 lines never executed by the current test suite. CLI command handlers (commands/lp.ts, commands/lending.ts, commands/swap.ts, ...) are mostly untested. This is the next-cycle P0 target. 2. defi-protocols statements 98.4% / functions 9.1% (1 of 11). The functions count under-reports because v8's provider only counts module-level functions, not adapter-class instance methods. The high statements % is real — module loading evaluates most adapter files — but branches at 27.8% reveals that conditional paths (e.g. cl_style switches, useTickSpacingQuoter) are largely untested. 3. defi-core 65.4% lines / 78.9% functions — the most balanced surface, fits the small-utility shape. Full report: docs/qa-reports/2026-05-05-coverage-baseline.md. Verified: monorepo build/test/lint clean — 79/79 tests pass on host (defi-core 32 + defi-protocols 18 + defi-cli 29; the defi-protocols delta vs PR #3 is because that PR is not yet merged here). Refs: P2 in docs/qa-reports/2026-05-05-test-foundation.md follow-ups.

PR #3 (feat/slippage-protection) closed 14 of the 15 sites that the 2026-05-05 baseline catalogued in qa/slippage.test.ts:KNOWN_INFINITE_SLIPPAGE. The line numbers also shifted when PR #4 (viem-chain anchor) refactored uniswap_v3.ts, so every entry in the original snapshot became stale on 2026-05-06 main and the test went red on every new PR (e.g. #7). This commit re-grounds the snapshot against current main: Adapter scan (regex unchanged): exactly 1 hit remains in the entire defi-protocols/src tree — dex/uniswap_v3.ts:262 amountOutMinimum: 0n (eth_call simulation inside quote() fallback; never broadcasts a tx) PR #3 hardened the other 14 sites: algebra_v3.ts:86,273,278,304 -> applyMinSlippage / explicit min args balancer_v3.ts:37 -> applyMinSlippage thena_cl.ts:78,165,190 -> applyMinSlippage / explicit min args uniswap_v3.ts:90,242,343,344, -> applyMinSlippage / explicit min args 363,364,403 In addition to refreshing the set, this commit fixes a self-trigger bug: uniswap_v3.ts:94 carries a documentation comment that warns against `amountOutMinimum: 0n`, and that comment matched the regex on its own — driving a false-positive "novel site" failure. Adding a `commentLinePattern` skip in both `it` blocks keeps the guard's intent (catch real broadcast paths) while letting docs reference the forbidden literal. Verified: - pnpm -C ts -r build — synced + dist rebuilt clean. - pnpm -C ts -r lint — 3 packages, tsc --noEmit clean. - pnpm -C ts -r test — 64/64 passing (was 62/64 before this fix). Refs: PR #7 CI failure (3x Build & Test red) was the smoke; root cause was the line-number drift, not anything Monad-related.

…rement (#30) P2 follow-up tracked in qa/2026-05-05-test-foundation. Establishes a quantitative coverage baseline so the next QA cycle can pick targets data-driven instead of guessing. Tooling changes: - ts/package.json: add @vitest/coverage-v8 ^3.0.0 (vitest official dev dep; npmjs.com/package/@vitest/coverage-v8). User-approved per SSOT Section 3. - ts/package.json: add `test:coverage` script that delegates to each workspace's own vitest run. Single root invocation reads 0% line coverage because the workspace dist alias intercepts source imports; per-package execution sees the actual src files. - ts/vitest.config.ts: workspace-package src aliases (used by future cross-package tests; root coverage path is preferred per-package). - packages/{defi-core,defi-protocols,defi-cli}/vitest.config.ts: per-package coverage block (provider=v8, text-summary reporter). - .gitignore: ts/coverage/ + ts/packages/*/coverage/. Baseline measurement (main @ ae5bb65): | package | lines | covered | % | |-----------------|------:|--------:|-------:| | defi-core | 399 | 261 | 65.4% | | defi-protocols | 5976 | 5880 | 98.4% | | defi-cli | 6299 | 961 | 15.3% | | **monorepo** | 12674 | 7102 | 56.0% | Headlines: 1. defi-cli at 15.3% — 5338 of 6299 lines never executed by the current test suite. CLI command handlers (commands/lp.ts, commands/lending.ts, commands/swap.ts, ...) are mostly untested. This is the next-cycle P0 target. 2. defi-protocols statements 98.4% / functions 9.1% (1 of 11). The functions count under-reports because v8's provider only counts module-level functions, not adapter-class instance methods. The high statements % is real — module loading evaluates most adapter files — but branches at 27.8% reveals that conditional paths (e.g. cl_style switches, useTickSpacingQuoter) are largely untested. 3. defi-core 65.4% lines / 78.9% functions — the most balanced surface, fits the small-utility shape. Full report: docs/qa-reports/2026-05-05-coverage-baseline.md. Verified: monorepo build/test/lint clean — 79/79 tests pass on host (defi-core 32 + defi-protocols 18 + defi-cli 29; the defi-protocols delta vs PR #3 is because that PR is not yet merged here). Refs: P2 in docs/qa-reports/2026-05-05-test-foundation.md follow-ups.

Hiksang added 7 commits May 5, 2026 16:27

Hiksang marked this pull request as ready for review May 5, 2026 09:16

Hiksang changed the title ~~feat(slippage): SSOT 7.3 fix — uniswap_v3 first (WIP, 3 adapters + CLI follow)~~ feat(slippage): SSOT 7.3 fix — 4 DEX adapters + CLI + 17 regression tests May 5, 2026

This was referenced May 5, 2026

hardening(viem-chain): anchor wallet/public clients to chainId (SSOT 7.4) #4

Merged

chore(coverage): coverage baseline + @vitest/coverage-v8 wiring #5

Merged

test(cli): unit tests for status + schema handlers (coverage P0 first step) #6

Merged

Hiksang merged commit 3a65f72 into main May 6, 2026
4 checks passed

Hiksang deleted the feat/slippage-protection branch May 6, 2026 08:58

Hiksang mentioned this pull request May 6, 2026

chore(qa): refresh slippage snapshot for post-PR3 reality #8

Merged

4 tasks

Hiksang restored the feat/slippage-protection branch May 8, 2026 11:04

Hiksang deleted the feat/slippage-protection branch May 8, 2026 11:58

Hiksang mentioned this pull request May 8, 2026

docs(qa): post-baseline test-foundation report (refined with monorepo-wide test counts) #31

Merged

Navigation Menu

Uh oh!

feat(slippage): SSOT 7.3 fix — 4 DEX adapters + CLI + 17 regression tests - #3

feat(slippage): SSOT 7.3 fix — 4 DEX adapters + CLI + 17 regression tests#3
Hiksang merged 7 commits into
mainfrom
feat/slippage-protection

Hiksang commented May 5, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

Hiksang commented May 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes by adapter

Types changes (commit 475e5e2)

CLI changes (commit 0d86ecd)

Tests added (commits bca992b + 31d069c)

Test plan

Coordination with PR #2

Commit history

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Hiksang commented May 5, 2026 •

edited

Loading

Types changes (commit `475e5e2`)

CLI changes (commit `0d86ecd`)

Tests added (commits `bca992b` + `31d069c`)