Phoenix-Protocol-Group / phoenix-contracts

Source code of the smart contracts of the Phoenix DeFi hub DEX protocol
GNU General Public License v3.0
10 stars 7 forks source link

PHOAM-007: Wrong calculation of swap constant reduces stable pool's efficiency #366

Closed gangov closed 4 months ago

gangov commented 4 months ago

Location

./contracts/pool_stable/src/math.rs:127

Description An error in the computation of a term when calculating the stable pools' constant reduces the stability range since the calculated value is smaller than expected. The calculation of the leverage_sub intermediate variable uses the wrong parameters:

/// Helper function used to calculate the D invariant as a last step in the `compute_d`public function.
///
/// * **Equation**:
///
/// d = (leverage * sum_x + d_product * n_coins) * initial_d /
((leverage - 1) * initial_d + (n_coins + 1) * d_product)
fn calculate_step(
env: &Env,
initial_d: &U256,
leverage: &U256,
sum_x: &U256,
d_product: &U256,
) -> U256 {
  // (leverage * sum_x + d_product * n_coins)
  let leverage_mul = leverage.mul(sum_x);
  let d_p_mul = d_product.mul(&U256::from_u128(env, N_COINS));
  let l_val = leverage_mul.add(&d_p_mul).mul(initial_d);
  // (leverage - 1) * initial_d
  let leverage_sub = leverage_mul.add(&leverage.sub(&U256::from_u128(env, 1)));
...
}

This expression does not follow the formula stated in the comment and performs a different calculation. The correct formula, as described in the comment, should be:

let leverage_sub = initial_d.mul(&leverage.sub(&U256::from_u128(env, 1)));

For instance, a protocol that uses the same curve for the AMM is Curve, and performs this calculation as follows

D = (Ann * S / A_PRECISION + D_P * N_COINS) * D / ((Ann - A_PRECISION) * D / A_PRECISION + (N_COINS + 1) * D_P)

Coinspect simulated the results of the constant calculation using both the current expression and a revised one based on the amended expression. The simulation showed that the current implementation results in a lower constant value compared to the revised expression. This affects to the pool's stability range because the wrongly calculated value for D is smaller than expected. The stable pool's curve respects the following expression for a two-token pool.

When graphing this curve in an X-Y axis, we get the following curves. The purple curve represents a pool with a higher D constant (calculated following the correct formula), whereas the red curve represents one with a smaller value (wrongly calculated, leading to a stability range loss).

When making a swap that increases the X amount in dX (for example from X = 10 offering 1 then X + dX = 11), the point over the curve tends to fall away from the stability range (the "flat" part of the curve) for those that have a smaller D. As a consequence, users will be making swaps that are not close to the 1:1 ratio they should.

Recommendation Fix how the curve's D is calculated.