osmosis icon indicating copy to clipboard operation
osmosis copied to clipboard

[x/gamm][stableswap]: Add tests for core pool joining logic (`JoinPool`, `CalcJoinPoolShares`, and `joinPoolSharesInternal`)

Open AlpinYukseloglu opened this issue 3 years ago • 1 comments

Background

Our current implementation of stableswap does not have tests for the core logic related to pool joining. Most of the core logic is in CalcJoinPoolShares, so I focused my example test cases/categories mainly on that function. My suggestions below should get this core logic in the ballpark of what we have for our balancer tests, but we should still make an effort to cover as many additional edge cases as possible.

Core logic to be tested: https://github.com/osmosis-labs/osmosis/blob/08669da8509059980dc964976ee1ca60c84f5c8a/x/gamm/pool-models/stableswap/amm.go#L377-L405

Suggested Design

Core cases to test:

  • single asset joins
  • multi asset exact ratio joins
  • multi asset non-exact ratio joins (both with large difference and with dust)

Specific things to test/errors to catch:

  • invalid length of tokensin (greater than 1, less than pool size should fail)
  • liquidity updates (keep in mind that CalcJoinPoolShares updates pool state in stableswap, unlike our balancer implementation)
  • correct number of output shares
  • for CalcJoinPoolShares (which just calls joinPoolSharesInternal), the number of tokens joined (both vs. expected amount and vs. actual tokens added to pool)

Acceptance Criteria

  • Existing tests pass and mutation tests on joinPoolSharesInternal show full coverage

AlpinYukseloglu avatar Aug 10 '22 08:08 AlpinYukseloglu

k derivation for tests: https://github.com/osmosis-labs/osmosis/issues/1368#issuecomment-1250743384

AlpinYukseloglu avatar Sep 19 '22 08:09 AlpinYukseloglu

Some multi-asset joins are blocked on precision increase in #3008

AlpinYukseloglu avatar Oct 16 '22 03:10 AlpinYukseloglu