Check swap slippage bounds against a live quote - #14
Merged
Conversation
`--max-usdc` was passed straight through as `maxUsdcAmount` with no comparison to anything, so a hand-rounded number was accepted in silence. That happened: a $56 bound was submitted against a ~$51.22 quote — a 9.3% tolerance on an illiquid pool, and since the bound governs the swap amount with fees landing on top, it authorised roughly $58.80 against a $53.78 expected debit. Every swap now prices a FRESH quote first and reports the tolerance the bound implies. `--max-slippage-pct` derives the bound exactly, instead of asking anyone to pick a number by hand. Bounds further than 5% from the quote are refused unless --allow-high-slippage. That threshold is pinned to the platform's own fee load (platform 3% + pool LP 2%) rather than chosen by taste: a tolerance wider than the entire fee schedule is far likelier a typo than an intention. Notably 10% — the first threshold considered — would NOT have caught the 9.2% case that prompted this. The check deliberately applies under --force as well. That is where it matters most: no prompt is shown, so a loose bound would otherwise pass unseen, which is exactly how the original bound was submitted. `implied_slippage_pct` is pure and unit-tested, including that a bound BETTER than the quote reports 0 rather than negative slippage, and that buys and sells bound in opposite directions. Claude-Session: https://claude.ai/code/session_01J7bz8aijn2Uwv1yF5rUenW
piekstra-dev
approved these changes
Jul 29, 2026
piekstra-dev
left a comment
piekstra-dev
left a comment
Collaborator
There was a problem hiding this comment.
Automated PR Review
Reviewed commit: a7f2fff279e2
Profile: reviewer - Posting as: piekstra-dev
Summary
| Reviewer | Findings |
|---|---|
| policies:conventions | 0 |
| documentation:docs | 0 |
Reviewer Coverage
| Reviewer | Status | Inspected | Skipped | Constraints |
|---|---|---|---|---|
| policies:conventions | complete_broad | README.md, src/commands/amm.rs | unavailable | cli-common shared docs (https://github.com/open-cli-collective/cli-common/tree/main/docs) and .github automation are not present in the review context, so no comparison against those source-of-truth docs was made |
| documentation:docs | complete_broad | README.md | unavailable | unavailable |
0 PR discussion threads considered. 0 summarized; 0 resolved.
Completed in 2m 52s | $1.83 | claude-sonnet-5 | cr 0.10.268
| Field | Value |
|---|---|
| Model | claude-sonnet-5 |
| Reviewers | policies:conventions, documentation:docs |
| Engine | claude_cli · claude-sonnet-5 |
| Reviewed by | cr · piekstra-dev |
| Duration | 2m 52s wall · 4m 33s compute |
| Cost | $1.83 |
| Tokens | 78 in / 16.7k out |
Per-workstream usage
| Workstream | Model | In | Out | Cache read | Cache create | Cost | Duration |
|---|---|---|---|---|---|---|---|
| orchestrator-selection | claude-sonnet-5 | 6 | 1.9k | 36.8k | 20.5k | $0.16 | 28s |
| policies:conventions | claude-sonnet-5 | 38 | 9.0k | 870.3k | 85.0k | $0.91 | 2m 12s |
| documentation:docs | claude-sonnet-5 | 28 | 5.4k | 473.3k | 69.5k | $0.64 | 1m 43s |
| orchestrator-rollup | claude-sonnet-5 | 6 | 392 | 59.4k | 15.8k | $0.12 | 8s |
piekstra
added a commit
that referenced
this pull request
Jul 29, 2026
Both rails in this release exist because the missing guard actually cost money on a live account, not because they seemed prudent: - `quote recenter|provision`: `--min-ask` now DEFAULTS to the position's break-even (#13). It was optional, so it only applied when remembered — and it got forgotten: a below-cost ask sat resting, filled, and locked in a loss. - `amm swap`: slippage bounds are checked against a fresh quote and refused above 5% (#14). `--max-usdc` was passed through unexamined, so a hand-rounded $56 on a ~$51 quote authorised ~9% slippage in silence. `--max-slippage-pct` derives the bound instead of asking anyone to guess. Both fire under --force, which is where they matter: no prompt is shown, so a loose value would otherwise pass unseen. Claude-Session: https://claude.ai/code/session_01J7bz8aijn2Uwv1yF5rUenW
piekstra
added a commit
that referenced
this pull request
Jul 30, 2026
`--max-usdc` was passed straight through as `maxUsdcAmount` with no comparison to anything, so a hand-rounded number was accepted in silence. That happened: a $56 bound was submitted against a ~$40.00 quote — a 10.0% tolerance on an illiquid pool, and since the bound governs the swap amount with fees landing on top, it authorised roughly $46.20 against a $42.00 expected debit. Every swap now prices a FRESH quote first and reports the tolerance the bound implies. `--max-slippage-pct` derives the bound exactly, instead of asking anyone to pick a number by hand. Bounds further than 5% from the quote are refused unless --allow-high-slippage. That threshold is pinned to the platform's own fee load (platform 3% + pool LP 2%) rather than chosen by taste: a tolerance wider than the entire fee schedule is far likelier a typo than an intention. Notably 10% — the first threshold considered — would NOT have caught the 10.0% case that prompted this. The check deliberately applies under --force as well. That is where it matters most: no prompt is shown, so a loose bound would otherwise pass unseen, which is exactly how the original bound was submitted. `implied_slippage_pct` is pure and unit-tested, including that a bound BETTER than the quote reports 0 rather than negative slippage, and that buys and sells bound in opposite directions.
piekstra
added a commit
that referenced
this pull request
Jul 30, 2026
Both rails in this release exist because the missing guard actually cost money on a live account, not because they seemed prudent: - `quote recenter|provision`: `--min-ask` now DEFAULTS to the position's break-even (#13). It was optional, so it only applied when remembered — and it got forgotten: a below-cost ask sat resting, filled, and locked in a loss. - `amm swap`: slippage bounds are checked against a fresh quote and refused above 5% (#14). `--max-usdc` was passed through unexamined, so a hand-rounded $56 on a ~$51 quote authorised ~9% slippage in silence. `--max-slippage-pct` derives the bound instead of asking anyone to guess. Both fire under --force, which is where they matter: no prompt is shown, so a loose value would otherwise pass unseen.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The gap
--max-usdcwas passed straight through to the API asmaxUsdcAmountwith no comparison to anything. A lazily rounded number was accepted in silence.That happened on a live account: a $56 bound against a ~$40.00 quote — a 10.0% tolerance on an illiquid pool. And because the bound governs the swap amount with fees landing on top, it authorised roughly $46.20 against a $42.00 expected debit.
What changed
Every
amm swapnow prices a fresh quote first and reports the tolerance the bound implies:--max-slippage-pct <PCT>derives the bound from the quote, so nobody has to pick a number by hand.--allow-high-slippage.Why 5%, specifically
Pinned to the platform's own fee load (3% platform + 2% pool LP), not chosen by taste: a slippage tolerance wider than the entire fee schedule is far likelier a typo or a rounded guess than an intention.
Worth noting 10% — the first threshold I tried — would NOT have caught the 10.0% case that prompted this. Picking a round number would have shipped a rail that missed the exact bug it was built for.
It applies under
--forceDeliberately. That's where it matters most: with no prompt shown, a loose bound passes unseen — which is precisely how the original $56 was submitted. Verified against the real command:
Testing
implied_slippage_pctis extracted as a pure function with tests covering the real $56/$40.00 case, sells bounding in the opposite direction, a bound better than the quote reporting 0 rather than negative slippage, and a zero reference. 97 tests,fmt, andclippy -D warningsclean.