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.

Sign up for GitHub

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

Jump to bottom

[x/gamm][stableswap]: Clean up rounding for single-asset joins #3355

Closed
Tracked by #3328
, #1451
...
AlpinYukseloglu opened this issue Nov 12, 2022 · 0 comments · Fixed by #3363
Closed
Tracked by #3328
, #1451
...

[x/gamm][stableswap]: Clean up rounding for single-asset joins #3355

AlpinYukseloglu opened this issue Nov 12, 2022 · 0 comments · Fixed by #3363
Assignees
@AlpinYukseloglu
Labels
C:stableswap C:x/gamm Changes, features and bugs related to the gamm module.

Comments

@AlpinYukseloglu
Copy link
Contributor

Background

We handle rounding for single asset joins quite oddly and should clean everything up to use our new rounding-specific comparison functions.

Suggested Design

  • Add rounding logic to ErrTolerance's CompareRounding function as is implemented in CompareBigDec
  • Replace Compare in BinarySearchSingleAssetJoin with CompareRounding
  • Remove the subtraction by 1 on the input to BinarySearchSingleAssetJoin
  • Remove additive error tolerance var and pass sdk.OneInt() in directly for ErrTolerance passed into BinarySearchSingleAssetJoin

Acceptance Criteria

  • All existing tests pass, and manually increasing additive error tolerance to 2 does not break anything
@AlpinYukseloglu AlpinYukseloglu added C:x/gamm Changes, features and bugs related to the gamm module. C:stableswap labels Nov 12, 2022
@AlpinYukseloglu AlpinYukseloglu self-assigned this Nov 12, 2022
@osmo-bot osmo-bot moved this to Needs Review 🔍 in Osmosis Chain Development Nov 12, 2022
This was referenced Nov 12, 2022
Closed
Merged
Closed
Closed
Repository owner moved this from Needs Review 🔍 to Done ✅ in Osmosis Chain Development Nov 14, 2022
@AlpinYukseloglu AlpinYukseloglu mentioned this issue Nov 14, 2022
Merged
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@AlpinYukseloglu AlpinYukseloglu

Labels
C:stableswap C:x/gamm Changes, features and bugs related to the gamm module.
Projects
Osmosis Chain Development
Archived in project
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[x/gamm][stableswap]: Clean up rounding for single-asset joins osmosis-labs/osmosis
1 participant
@AlpinYukseloglu