zeitgeist icon indicating copy to clipboard operation
zeitgeist copied to clipboard

Some swaps return `MathApproximation` error for valid values

Open sea212 opened this issue 1 year ago • 1 comments

Minimal test for swaps pallet:

#[test]
fn swap_in_math_approximation_occurs_for_small_values() {
    ExtBuilder::default().build().execute_with(|| {
        let creator_fee = Perbill::from_percent(1);
        let swap_fee = 0;
        let asset_amount = (BASE / 10_000) - 1;
        create_initial_pool_with_funds_for_alice(ScoringRule::CPMM, Some(swap_fee), true);

        assert_ok!(Swaps::swap_exact_amount_in(
            alice_signed(),
            DEFAULT_POOL_ID,
            ASSET_A,
            asset_amount,
            ASSET_D,
            Some(0),
            None,
        ));
    });
}

Swapping the same value as asset_amount_out in swap_exact_amount_out works:

#[test]
fn swap_out_math_approximation_does_not_occur() {
    ExtBuilder::default().build().execute_with(|| {
        let creator_fee = Perbill::from_percent(1);
        let swap_fee = 0;
        let asset_amount = (BASE / 10_000) - 1;
        create_initial_pool_with_funds_for_alice(ScoringRule::CPMM, Some(swap_fee), true);

        assert_ok!(Swaps::swap_exact_amount_out(
            alice_signed(),
            DEFAULT_POOL_ID,
            ASSET_A,
            Some(u128::MAX),
            ASSET_D,
            asset_amount,
            None,
        ));
    });
}

sea212 avatar Sep 08 '23 14:09 sea212

Oh, nice one. Fails on this check:

    match p.pool.scoring_rule {
        ScoringRule::CPMM => ensure!(
            spot_price_before
                <= bdiv(asset_amount_in.saturated_into(), asset_amount_out.saturated_into())?
                    .saturated_into(),
            Error::<T>::MathApproximation
        ),
        ScoringRule::RikiddoSigmoidFeeMarketEma => {
            // Currently the only allowed trades are base_currency <-> event asset. We count the
            // volume in base_currency.
            let base_asset = p.pool.base_asset;
            let volume = if p.asset_in == base_asset { asset_amount_in } else { asset_amount_out };
            T::RikiddoSigmoidFeeMarketEma::update_volume(p.pool_id, volume)?;
        }
    }

amount_in is 999_999 but should be at least 1_000_000, while amount_out is (as expected) 100_000, resulting in a fraction amount_in / amount_out of 9_999_990_000, while the previous spot price is 10_000_000_000.

Solution is probably to implement proper rounding behavior.

maltekliemann avatar Sep 08 '23 14:09 maltekliemann