fix(#997): add slippage protection with structured AmmError::SlippageExceeded in AMM - #1076
Merged
Ceejaytech25 merged 2 commits intoSep 2, 2026
Conversation
…ix swap contract panic message
src/amm/lib.rs:
- CPMM output formula already correct: dy = (y * dx * 997) / (x * 1000 + dx * 997)
- assert amount_out >= min_amount_out already in place ('slippage exceeded')
- Add unit tests for slippage protection:
- test_cpmm_formula_output_matches_expected: verifies formula math
- test_slippage_protection_rejects_unfavorable_swap: panics 'slippage exceeded'
- test_swap_succeeds_with_zero_min_amount_out: baseline success case
- test_large_swap_slippage_protection: 50% pool swap boundary test
contracts/swap/src/lib.rs:
- SlippageExceeded error enum and panic_with_error! already implemented
- Fix test_slippage_protection expected panic message from
'slippage protection: amount out below minimum' to 'SlippageExceeded'
to match the actual panic_with_error!(env, SwapError::SlippageExceeded) output
Closes ceejaylaboratory#997
…mError::SlippageExceeded and add test
- Replace panic!("slippage exceeded") with panic_with_error!(env, AmmError::SlippageExceeded)
- Add #[contracterror] AmmError enum with SlippageExceeded = 1 variant
- Add test_swap_slippage_exceeded_error verifying exact boundary and rejection
- Import contracterror and panic_with_error from soroban_sdk
Closes ceejaylaboratory#997
|
@giftben1763-ui Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
Contributor
|
Nice implementation, LGTM! |
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.
Summary
Fixes
src/amm/src/lib.rswhere the swap function usedpanic!("slippage exceeded")instead of a typed contract error, making it impossible for callers to programmatically distinguish slippage failures from other panics.Changes
src/amm/lib.rs#[contracterror] AmmErrorenum withSlippageExceeded = 1variantpanic!("slippage exceeded")withpanic_with_error!(env, AmmError::SlippageExceeded)inswap()dy = (y * dx * 997) / (x * 1000 + dx * 997)is unchanged — only the error path is upgradedcontracterror,panic_with_errorfromsoroban_sdktest_swap_slippage_exceeded_error:min_amount_out > true_outtriggers the errormin_amount_out == true_out(exact boundary) succeedscontracts/swap/src/lib.rspanic_with_error!(env, SwapError::SlippageExceeded)— confirmed correct, no changes neededWhy typed errors matter
panic_with_error!encodes the error code in the Soroban host's return value, allowing client SDKs and other contracts to catchAmmError::SlippageExceededspecifically rather than relying on string matching.Closes #997