Assessment reports>Liquidswap>Low findings>Incorrect rounding behavior in ,router::get_coin_in_with_fees
Category: Coding Mistakes

Incorrect rounding behavior in router::get_coin_in_with_fees

Low Severity
Low Impact
Low Likelihood

Description

In the function router::get_coin_in_with_fees, the result is rounded up incorrectly for both stable and uncorrelated curves, which can lead to an undue amount being paid in fees.

The formula for rounding up integer division is (n - 1)/d + 1 for n > 0.

let coin_in = (stable_curve::coin_in(
    (coin_out as u128),
    scale_out,
    scale_in,
    (reserve_out as u128),
    (reserve_in as u128),
) as u64) + 1;

(coin_in * fee_scale / fee_multiplier) + 1

The stable curve branch of router::get_coin_in_with_fees does not correctly implement the formula stated above.

let coin_in = math::mul_div(
    coin_out, // y
    reserve_in * fee_scale, // rx * 1000
    new_reserves_out   // (ry - y) * 997
) + 1;

Furthermore, the uncorrelated curve branch also incorrectly implements the formula stated above.

Impact

For certain swap amounts, a user could end up paying more in fees than would be accurate.

Recommendations

In the case of the stable curve branch of router::get_coin_in_with_fees, the code should be rewritten to adhere to the rounded up integer division formula.

let coin_in = (stable_curve::coin_in(
    (coin_out as u128),
    scale_out,
    scale_in,
    (reserve_out as u128),
    (reserve_in as u128),
) as u64);

let n = coin_in * fee_scale;

if (n > 0) {
    ((n - 1) / fee_multiplier) + 1
} else {
    0
}

Likewise, the uncorrelated curve branch also needs a revision.

// add to liquidswap::math
public fun mul_div_rounded_up(x: u64, y: u64, z: u64): u64 {
    assert!(z != 0, ERR_DIVIDE_BY_ZERO);
    let n = (x as u128) * (y as u128);
    let r = if (n > 0) {
        ((n - 1) / (z as u128)) + 1
    } else {
        0
    }
    (r as u64)
}

let coin_in = math::mul_div_rounded_up(
    coin_out, // y
    reserve_in * fee_scale, // rx * 1000
    new_reserves_out   // (ry - y) * 997
);

Remediation

Pontem Network fixed this issue in commit 0b01ed6

Zellic © 2023Back to top ↑