-
Notifications
You must be signed in to change notification settings - Fork 609
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 boilerplate for stableswap #1214
Conversation
|
||
func ValidateFutureGovernor(governor string) error { |
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.
This function was duplicated, and existed in the types package.
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.
Curious, why the decision to have a sub-package called under x/gamm
? Instead of:
- directly in x/gamm, OR
- separate module entirely
]; | ||
} | ||
|
||
message Pool { |
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.
nit: proto doc :p
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.
Not really sure what doc to put here yet, I think It'll become clearer when I go to make one go-side after an initial impl of the math. Added a placeholder for now though
Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com>
Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com>
Codecov Report
@@ Coverage Diff @@
## main #1214 +/- ##
==========================================
- Coverage 21.03% 20.99% -0.05%
==========================================
Files 196 196
Lines 25349 25345 -4
==========================================
- Hits 5332 5320 -12
- Misses 19054 19066 +12
+ Partials 963 959 -4
Continue to review full report at Codecov.
|
I'm down for changing how we do the proto package structure. I don't know whats standard. Go-side, I think the Don't want to be in a situation where we have |
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!
I also agree with @ValarDragon that we maintain the sub-package structure where different gamm models are structured like x/gamm/pool-models/balancer
and x/gamm/pool-models/stableswap
. One of the reasons I'm in support of this structure is that go import cycle can be rather tricky and keeping a sub-package is helpful in managing this import cycle structure between packages.
Lets discuss package structure in a separate issue, going to go ahead and merge this to simplify rebasing further logic work on stableswap! |
Adds boilerplate proto definitions for stableswap. This should not affect the current state machine
For contributor use:
docs/
) or specification (x/<module>/spec/
)Unreleased
section inCHANGELOG.md
Files changed
in the Github PR explorer