-
Notifications
You must be signed in to change notification settings - Fork 602
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
Remove osmomath's dependency on osmoassert & gammtypes #3768
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
// but we can't directly import that due to import cycles. | ||
// Hence we use the same var name, in hopes that if any change there happens, | ||
// this is caught via a CTRL+F | ||
var MaxSpotPrice = sdk.NewDec(2).Power(128).Sub(sdk.OneDec()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would having separate constants or utils package help mitigate the import cycles?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would have to be in its own independent go.mod, which seems pretty annoying for now
* Remove osmomath's dependency on osmoassert & gammtypes * Add note in gamm constant * Bring pack conditional panic * One more cond panic (cherry picked from commit db97b16)
* Remove osmomath's dependency on osmoassert & gammtypes * Add note in gamm constant * Bring pack conditional panic * One more cond panic (cherry picked from commit db97b16) Co-authored-by: Dev Ojha <[email protected]>
What is the purpose of the change
Needed to split osmomath into its own go.mod
This basically involved making an exported var for
gammtypes.MaxSpotPrice
, and removing Conditional Panic cases. Only affects test files.