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

Fix stableswap use of osmomath #3909

Merged
merged 3 commits into from
Jan 3, 2023
Merged

Fix stableswap use of osmomath #3909

merged 3 commits into from
Jan 3, 2023

Conversation

nicolaslara
Copy link
Contributor

What is the purpose of the change

2ac5d35 breaks stableswap because osmomath requires the users of the binsearch functions to use the name input for the function's argument. This removes that requirement

Brief Changelog

Make function used by binary search on osmomath more flexible

Testing and Verifying

All tests continue to pass

Documentation and Release Note

  • Does this pull request introduce a new feature or user-facing behavior changes? (yes / no)
  • Is a relevant changelog entry added to the Unreleased section in CHANGELOG.md? (yes / no)
  • How is the feature or change documented? (not applicable / specification (x/<module>/spec/) / Osmosis docs repo / not documented)

@nicolaslara nicolaslara added the V:state/compatible/no_backport State machine compatible PR, depends on prior breaks label Jan 3, 2023
Copy link
Member

@p0mvn p0mvn left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

@@ -144,7 +144,7 @@ func (e ErrTolerance) CompareBigDec(expected BigDec, actual BigDec) int {
// Binary search inputs between [lowerbound, upperbound] to a monotonic increasing function f.
// We stop once f(found_input) meets the ErrTolerance constraints.
// If we perform more than maxIterations (or equivalently lowerbound = upperbound), we return an error.
func BinarySearch(f func(input sdk.Int) (sdk.Int, error),
func BinarySearch(f func(sdk.Int) (sdk.Int, error),
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this makes a difference because #3904 tags main directly and passes CI

@p0mvn p0mvn merged commit f611e01 into main Jan 3, 2023
@p0mvn p0mvn mentioned this pull request Jan 3, 2023
@p0mvn p0mvn deleted the nicolas/fix-stableswap-osmomath branch January 3, 2023 15:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
V:state/compatible/no_backport State machine compatible PR, depends on prior breaks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants