gear-tech / apps

Gear Apps
GNU General Public License v3.0
8 stars 9 forks source link

Add StableSwap DEX (CurveAmm) example. #15

Closed vivekvpandya closed 2 years ago

vivekvpandya commented 2 years ago

Resolves #11 .

LouiseMedova commented 2 years ago

Explain the following to me please: 1) The action create_pool: what will be if I create a pool with assets that are already exist in another pool? For what purpose you check pools for incosistent storage? Pool_id is taken from struct CurveAmm and in the end you increase it by one. 2) The action add_liquidity: I think that you should check that the pool with the indicated pool_id exists. Then, how can be n_coins != pool.assets.len() if one line higher you write let n_coins = pool.assets.len()? Can you please describe what steps you are doing in that function to make clear all the calculations?

LouiseMedova commented 2 years ago

@vivekvpandya Please explain what is going on in the function add_liquidity. I understood that first of all you calculate the stableSwap invariant D for the old liquidity, then you add tokens to the pool and calculate that invariant for the new liquidity, that is clear. But then you recalculate D for fees and I don't understand the following calculations.

vivekvpandya commented 2 years ago

Explain the following to me please:

1. The action `create_pool`: what will be if I create a pool with assets that are already exist in another pool?
   For what purpose you check `pools` for `incosistent storage`? `Pool_id` is taken from `struct CurveAmm` and in the end you increase it by one.

There can be more than one pool having same assets (but different configuration for fees, etc..) . Each time a pool is created a new id is assiged (which is incremental in nature so say there are 10 pools exists then their ids are in range [0,9] next pool will be assigned id = 10). self.pool_count = pool_id.checked_add(1).expect("inconsistent storage"); here inconsistent storage is mistake, I will change it to math error.

2. The action `add_liquidity`: I think that you should check that the `pool` with the indicated `pool_id` exists.
   Then, how can be `n_coins != pool.assets.len()` if one line higher you write `let n_coins = pool.assets.len()`?

this also silly check. I will remove. Can you please describe what steps you are doing in that function to make clear all the calculations?

vivekvpandya commented 2 years ago

@vivekvpandya Please explain what is going on in the function add_liquidity. I understood that first of all you calculate the stableSwap invariant D for the old liquidity, then you add tokens to the pool and calculate that invariant for the new liquidity, that is clear. But then you recalculate D for fees and I don't understand the following calculations.

I have kept that code from equilibrium-finanace's code, it is required to charge fees in add_liquidity() because other wise there can be some application level attacks where you can do a swap without paying any fees to pool. However I feel that we can drop that code for now and reintroduce it when I have more concrete examples and data for such attacks.

For now I have following references to study: https://arxiv.org/pdf/2103.12732.pdf https://uniswap.org/whitepaper.pdf

shamilsan commented 2 years ago

Stale PR, may we close it?

LouiseMedova commented 2 years ago

Stale PR, may we close it?

Yes I think we should close it