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

Liquidity balancer pool PokeTokenWeights and params implementation #103

Merged
merged 19 commits into from
Apr 19, 2021

Conversation

ValarDragon
Copy link
Member

@ValarDragon ValarDragon commented Apr 17, 2021

This PR is ready for review. The remaining TODO's should come in follow-on PRs.

  • Updates for LBP logic to pool proto structs
  • Remove PoolAccountI SetTokenWeight, unused, and done in favor of a new PokeWeights method and assuming only smooth transitions
  • Remove PoolAccountI SetPoolParams - unused, and should be remade with pool governance in mind. The alternative would be to build validation for it right now.
  • Fix bug where future_pool_governor would not get serialized into JSON/YAML
  • Arithmetic for PokeTokenWeights implemented
  • Tests for PokeTokenWeights
  • Remove poolWeightsChanging from PoolAccount (Done, but was a change internal to this PR)
  • (DONE, Precision limits for gamm #112): Change token weights to have precision bounds, and scale them internally
  • (Done - reuses same message w/ validation Add validation for LBP params #128 ): Add new LBP message type, or ensure it works with existing message
  • (Done, series of PRs): Add LBP CLI tx type
  • (Future PR): Efficiency Optimization to make PokeTokenWeights only get called at most once / block
  • (Done, Delete Smooth param change params from pool params when done #122 ): Efficiency Optimization to make SmoothWeightChangeParams get set to nil once done

@ValarDragon
Copy link
Member Author

ValarDragon commented Apr 17, 2021

Something unforeseen thats needed is we will need some precision bounds on the weights in order to capture intermediate weights for small sized weights. My plan is to cap the size of user-provided weights at 2^20, then shift these internally to 2^50, giving 30 bits or ~9 decimal places of precision for natural weights to smoothly change over.

This way, even if we switch to capping weights as being measured in uint64's, we can still support up to 2^13 = 8192 assets per pool.

However, I'd like to do this in a follow-on PR after this one, for ease of review. I'll write the tests for PokeTokenWeights with already scaled weights for now.

@ValarDragon ValarDragon changed the title Liquidity balancer pool Liquidity balancer pool PokeTokenWeights and params implementation Apr 17, 2021
@ValarDragon ValarDragon marked this pull request as ready for review April 17, 2021 19:22
@ValarDragon
Copy link
Member Author

This PR is now completely ready for review.

Copy link
Collaborator

@Thunnini Thunnini left a comment

Choose a reason for hiding this comment

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

Overall it looks good.
But JoinSwapExternAmountIn, ExitSwapShareAmountIn, ExitSwapExternAmountOut all gets affected by the weight so PokeTokenWeights should be run first. Also, the results of the CalculateSpotPrice, CalculateSpotPriceSansSwapFee queries should consider LBP so I think PokeTokenWeights should be run first. Maybe it’d be more convenient to run PokeTokenWeights first and return the pool in GetPool().

x/gamm/types/account.go Outdated Show resolved Hide resolved
@sunnya97
Copy link
Collaborator

Maybe it’d be more convenient to run PokeTokenWeights first and return the pool in GetPool().

This could also be good because queries probably want the latest weights as well

@ValarDragon
Copy link
Member Author

Great idea! PokeTokenWeights has been moved to GetPool

@Thunnini Thunnini self-requested a review April 19, 2021 16:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants