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

[x/gamm][StableSwap] multi-asset swap / LP equations, and tests #1369

Closed
Tracked by #1451
ValarDragon opened this issue Apr 28, 2022 · 2 comments · Fixed by #1383 or #1429
Closed
Tracked by #1451

[x/gamm][StableSwap] multi-asset swap / LP equations, and tests #1369

ValarDragon opened this issue Apr 28, 2022 · 2 comments · Fixed by #1383 or #1429
Assignees

Comments

@ValarDragon
Copy link
Member

ValarDragon commented Apr 28, 2022

The current CFMM equation we use for stableswap is xy(x^2 + y^2) = k.

To extend this to many assets, we use the natural generalization, e.g. for 4 assets, xyzw(x^2 + y^2 + z^2 + w^2). To implement this, we seek to have a single CFMM equation for swaps that works for all numbers of assets. We achieve this via a variable substitution. Let u = zw and v = z^2 + w^2. Then the stableswap equation for a swap is the same for all numbers of assets, taking the natural generalization of the two. Namely:

xyu(x^2 + y^2 + v)

And we just make our swap CFMM algorithms relative to this.

@ValarDragon ValarDragon mentioned this issue Apr 28, 2022
6 tasks
@daniel-farina daniel-farina moved this to 🔍 Needs Review in Osmosis Chain Development Apr 28, 2022
@mergify mergify bot closed this as completed in #1383 May 4, 2022
mergify bot pushed a commit that referenced this issue May 4, 2022
Closes: #1369 

## What is the purpose of the change

This change extends the CFMM equation we use for stableswap to multiple assets.

## Brief change log
 
  - Create multi-asset CFMM function
  - [wip] Create multi-asset CFMM solver
  - [wip] Create tests for new CFMM


## Testing and Verifying

This change added tests and can be verified as follows:

*(example:)*
  - [wip]

## Documentation and Release Note

  - Does this pull request introduce a new feature or user-facing behavior changes? (yes)
  - 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](https://github.com/osmosis-labs/docs)   /   not documented)
Repository owner moved this from 🔍 Needs Review to ✅ Done in Osmosis Chain Development May 4, 2022
@ValarDragon ValarDragon reopened this May 4, 2022
Repository owner moved this from ✅ Done to 🏃 In Progress in Osmosis Chain Development May 4, 2022
@mergify mergify bot closed this as completed in #1429 May 6, 2022
mergify bot pushed a commit that referenced this issue May 6, 2022
Closes: #1369

## What is the purpose of the change

This PR implements the solver for our multi-asset CFMM (as outlined in the previous PR for issue #1369).

This is an expansion of the original two-asset CFMM we extended from Solidly, which we expand here to accommodate n-asset pools without significantly increasing complexity or runtime. While this type of multi-asset pool was one that we found was perhaps unnecessarily complex for volatile assets, it is critical from a capital efficiency standpoint that we have the option for Curve-like 3pools and 4pools to not fragment stablecoin liquidity (and therefore also LP incentives) within our stableswap, especially as we compete for stablecoin liquidity with many other stableswaps.

Our final multi-asset CFMM is `xyz(x^2 + y^2 + w) = k`, where z represents the product of the reserves of all assets that are not being swapped between, and w represents the sum of all of their squares. This PR implements a solver that finds the correct output for of a requested token `a` given an input of another token `b` in the pool and accommodates swaps between any two assets `x` and `y` in an n-asset stableswap. The polynomial we solve for can be described as:

`(x - a)(y + b)z((x - a)^2 + (y +b)^2 + w)`, where we solve for `a`. The solution is quite complex and not easy to visually parse without the abstractions we have laid out in the code comments, so please refer to those for more comprehensive and readable implementation details.

## Brief change log

  - Implemented `solveCfmmMulti` function in `x/gamm/pool-models/stableswap/amm.go` using the derivations set up in code comments
  - Implemented binary search logic for this new multi-asset CFMM
  - Set up tests for multi-asset pools and expanded existing two-asset stableswap tests to also check consistency between our two CFMM implementations
  - Cleaned up/simplified the math for the solver to make it more readable and simpler to compute (e.g. limiting the max bitlen to avoid risking integer overflows)

## Testing and Verifying

The tests for this change line up in coverage with those for our two-asset stableswap CFMM and can be found in `amm_test.go`.

## Documentation and Release Note

  - Does this pull request introduce a new feature or user-facing behavior changes? (yes)
  - Is a relevant changelog entry added to the `Unreleased` section in `CHANGELOG.md`? (no)
  - How is the feature or change documented? (not documented)
Repository owner moved this from In Progress🏃 to Done ✅ in Osmosis Chain Development May 6, 2022
@p0mvn p0mvn changed the title multi-asset swap / LP equations, and tests [x/gamm] multi-asset swap / LP equations, and tests May 9, 2022
@p0mvn p0mvn mentioned this issue May 9, 2022
55 tasks
@p0mvn p0mvn changed the title [x/gamm] multi-asset swap / LP equations, and tests [x/gamm][StableSwap] multi-asset swap / LP equations, and tests May 9, 2022
@p0mvn p0mvn reopened this May 9, 2022
Repository owner moved this from Done ✅ to In Progress🏃 in Osmosis Chain Development May 9, 2022
@p0mvn
Copy link
Member

p0mvn commented May 9, 2022

More work need to be done to get all the AMM logic use multi-asset liquidity. @ValarDragon please provide more context

@AlpinYukseloglu
Copy link
Contributor

Update: the only remaining part for this issue (tests for higher-level pool joining logic) is covered by #2346, so we can close this issue

Repository owner moved this from In Progress🏃 to Done ✅ in Osmosis Chain Development Oct 16, 2022
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
3 participants