Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[stableswap]: Add checks for invalid cfmm inputs #2695

Merged
merged 2 commits into from
Sep 14, 2022
Merged

Conversation

AlpinYukseloglu
Copy link
Contributor

Closes: #2694

What is the purpose of the change

From issue:

While testing our stableswap CFMMs (both two-asset and multi), I noticed that given their complexity it is possible for certain really weird inputs to get passed into the solvers that go completely uncaught. Both for security reasons and for more time-effective testing/bug squashing, I think we should implement validity checks for inputs to (and outputs of) the CFMM solvers.

This PR adds these validity checks.

Brief Changelog

  • Ensure inputs and outputs of CFMM functions are checked for basic validity (not negative, outputs not greater than reserves etc.)

Testing and Verifying

This change added tests that verify negative input checks in stableswap/amm_test.go

Documentation and Release Note

  • Does this pull request introduce a new feature or user-facing behavior changes? (no)
  • Is a relevant changelog entry added to the Unreleased section in CHANGELOG.md? (no)
  • How is the feature or change documented? (not documented)

@AlpinYukseloglu AlpinYukseloglu added the C:x/gamm Changes, features and bugs related to the gamm module. label Sep 10, 2022
@AlpinYukseloglu AlpinYukseloglu requested a review from a team September 10, 2022 21:55
Copy link
Member

@ValarDragon ValarDragon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice, ty!

x/gamm/pool-models/stableswap/amm_test.go Outdated Show resolved Hide resolved
x/gamm/pool-models/stableswap/amm_test.go Outdated Show resolved Hide resolved
x/gamm/pool-models/stableswap/amm_test.go Outdated Show resolved Hide resolved
x/gamm/pool-models/stableswap/amm_test.go Outdated Show resolved Hide resolved
@AlpinYukseloglu
Copy link
Contributor Author

I just went ahead and refactored the tests to fit our standards – I figured it would be easier to resolve issues in all the open PRs right now than try to fit in a full test refactor later

@p0mvn p0mvn merged commit 3841962 into main Sep 14, 2022
@p0mvn p0mvn deleted the stable-cfmm-checks branch September 14, 2022 20:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C:x/gamm Changes, features and bugs related to the gamm module.
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

[stableswap]: Add checks for invalid cfmm inputs
3 participants