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]: Prevent int overflows in intermediate CFMM calculations #2701

Closed
Tracked by #1451
AlpinYukseloglu opened this issue Sep 12, 2022 · 2 comments
Closed
Tracked by #1451
Labels
C:stableswap T:bug 🐛 Something isn't working

Comments

@AlpinYukseloglu
Copy link
Contributor

Background

Our CFMM solvers (even the simplified ones) have extremely large terms (^8) in them that cause int overflows for relatively small pools. It likely isn't possible to simplify the solvers much further in a way that would bound the largest term, so we will have to find another way to deal with extremely large intermediate terms.

Suggested Design

Some options might be:

  • Find a way to shift around terms such that the largest one is always bounded
  • Add a switch that, for very large inputs, removes extremely small terms to simplify logic and minimize large exponents

Acceptance Criteria

  • A stableswap pool with $100M in each asset should not overflow
@AlpinYukseloglu AlpinYukseloglu added T:bug 🐛 Something isn't working C:x/gamm Changes, features and bugs related to the gamm module. labels Sep 12, 2022
@osmo-bot osmo-bot moved this to Needs Review 🔍 in Osmosis Chain Development Sep 12, 2022
@AlpinYukseloglu AlpinYukseloglu changed the title [stableswap]: Bound int overflows in intermediate CFMM calculations [stableswap]: Prevent int overflows in intermediate CFMM calculations Sep 12, 2022
@AlpinYukseloglu
Copy link
Contributor Author

Looks like we might be able to bound the largest term (x^8 and k^2) like this:
image

Will continue trying to wrangle this to fit inside bounds but this overflow issue is looking more and more solvable

@mattverse mattverse added C:stableswap and removed C:x/gamm Changes, features and bugs related to the gamm module. labels Sep 13, 2022
@AlpinYukseloglu
Copy link
Contributor Author

Closed by #2777 and #2778

Repository owner moved this from Needs Review 🔍 to Done ✅ in Osmosis Chain Development Sep 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C:stableswap T:bug 🐛 Something isn't working
Projects
Archived in project
Development

No branches or pull requests

2 participants