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

[x/gamm][stableswap]: Add tests for single-asset joins (calcSingleAssetJoinShares) #2348

Closed
Tracked by #1451
AlpinYukseloglu opened this issue Aug 10, 2022 · 4 comments
Labels
C:x/gamm Changes, features and bugs related to the gamm module. T:tests
Milestone

Comments

@AlpinYukseloglu
Copy link
Contributor

AlpinYukseloglu commented Aug 10, 2022

Background

Our current implementation of calcSingleAssetJoinShares is untested. We should make tests that cover similar cases to what we have in balancer.

Suggested Design

  • Create single-asset pool join tests that mirror what we have in balancer but with our stableswap CFMM

Acceptance Criteria

  • All tests pass and removing updatePoolForJoin through mutation testing doesn't break calcSingleAssetJoinShares
@AlpinYukseloglu AlpinYukseloglu added T:tests C:x/gamm Changes, features and bugs related to the gamm module. labels Aug 10, 2022
@osmo-bot osmo-bot moved this to Needs Review 🔍 in Osmosis Chain Development Aug 10, 2022
@AlpinYukseloglu AlpinYukseloglu changed the title stableswap: Add tests for single-asset joins stableswap: Add tests for single-asset joins (calcSingleAssetJoinShares) Aug 10, 2022
@AlpinYukseloglu AlpinYukseloglu changed the title stableswap: Add tests for single-asset joins (calcSingleAssetJoinShares) [x/gamm][stableswap]: Investigate critical precision issue with binary search CFMM solver: Add tests for single-asset joins (calcSingleAssetJoinShares) Aug 10, 2022
@AlpinYukseloglu AlpinYukseloglu changed the title [x/gamm][stableswap]: Investigate critical precision issue with binary search CFMM solver: Add tests for single-asset joins (calcSingleAssetJoinShares) [x/gamm][stableswap]: Add tests for single-asset joins (calcSingleAssetJoinShares) Aug 25, 2022
@hieuvubk hieuvubk self-assigned this Aug 29, 2022
@hieuvubk hieuvubk removed their assignment Sep 26, 2022
@pysel
Copy link
Member

pysel commented Sep 30, 2022

Is this issue available to work on?

@p0mvn
Copy link
Member

p0mvn commented Sep 30, 2022

@rusakh Yes, please

You can get inspiration from the existing test cases in balancer. However, they might not be exhaustive so if you think of any extra, please add them

@p0mvn p0mvn added this to the V13 milestone Sep 30, 2022
@AlpinYukseloglu
Copy link
Contributor Author

I actually covered a significant portion of this issue in #2905, so it might not be a great one to pick up if you don't have context (properly testing this functionality is actually a relatively complicated process)

@AlpinYukseloglu
Copy link
Contributor Author

AlpinYukseloglu commented Oct 16, 2022

Update: single-asset joins are now tested (#2905) and can be further tested as part of #2900 and #2346

Repository owner moved this from Needs Review 🔍 to Done ✅ in Osmosis Chain Development Oct 16, 2022
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. T:tests
Projects
Archived in project
Development

No branches or pull requests

4 participants