Incorrect Fee Calculation Due to Precision Scaling in get_dy and get_dy_underlying

[hats-bug-reporter]

Github username: --
Twitter username: --
Submission hash (on-chain): 0xa91a2146e896185ba455a1882220d158a0a8a4f79096346c6c94bc61e5420089
Severity: high

Description:
Description
The current implementation of the get_dy and get_dy_underlying function in the StableSwapTwoPool.sol and StableSwapThreePool.sol contract calculates the swap fee after scaling the output amount dy to the token's native precision. This approach leads to incorrect fee calculations. The fee is calculated on a potentially smaller value, resulting in a lower fee than intended.

below is the get_dy() function:

 function get_dy(
        uint256 i,
        uint256 j,
        uint256 dx
    ) external view returns (uint256) {
        // dx and dy in c-units
        uint256[N_COINS] memory rates = RATES;
        uint256[N_COINS] memory xp = _xp();

        uint256 x = xp[i] + ((dx * rates[i]) / PRECISION);
        uint256 y = get_y(i, j, x, xp);
        uint256 dy = ((xp[j] - y - 1) * PRECISION) / rates[j];
        uint256 _fee = (fee * dy) / FEE_DENOMINATOR;//@audit-incorrect fee calculation
        return dy - _fee;
    }

lets understand issue with example
Attachments

1. Proof of Concept (PoC) File

Consider a scenario:

• The swap involves USDC, which has 6 decimals.
• The difference between virtual balances (xp[j] - y - 1) is 5e18 in the pool's internal precision (18 decimals).
• The swap fee is set to 1e9.
• FEE_DENOMINATOR is 1e10.

Current Implementation :

1. Calculate dy:

• dy = (5e18 * 1e18) / 1e30 = 5e6 (scaled to 6 decimals).

2. Calculate Fee:

• _fee = (1e9 * 5e6) / 1e10 = 5e5.

3. Net Amount:

• dy - _fee = 5e6 - 5e5 = 4.5e6.
  Fee Received:

The fee received here is 5e5.

CORRECT Implementation:

1. Calculate dy:

• dy = 5e18 (remains in 18 decimals).

2. Calculate Fee:(here fee is calculated before precision handling)

• dy_fee = (5e18 * 1e9) / 1e10 = 5e17.

3. Net Amount:

• dy - dy_fee/precision = (5e18 - 5e17)/5e12 = 4.5e6.

4. Fee Received:

The fee received is 5e17 .

as we can see above the fee received in the current implementation is significantly less for equal amount of tokens

2. Revised Code File (Optional)

• To resolve this issue, the fee should be calculated before scaling dy to the token's native precision.

function get_dy(
    uint256 i,
    uint256 j,
    uint256 dx
) external view returns (uint256) {
    // dx and dy in c-units
    uint256[N_COINS] memory rates = RATES;
    uint256[N_COINS] memory xp = _xp();

    uint256 x = xp[i] + ((dx * rates[i]) / PRECISION);
    uint256 y = get_y(i, j, x, xp);
    uint256 dy = xp[j] - y - 1; 
    uint256 dy_fee = (fee * dy) / FEE_DENOMINATOR;//calculating fee
    // Subtract the fee and scale dy to the token's native precision
    dy = (dy - dy_fee) * PRECISION / rates[j];
    return dy;
}

• this issue is present in both the 2-pool and 3-pool,so make sure to fix the issue in both contracts

[omega-audits]

The fee is calculated correctly and as intended as the calculations show

[batmanBinary]

hello @omega-audits ,can u please elaborate as intended as the calculations show?

as far as i can see,there is a huge difference in the current fee calculation and the expected fee calculation.

current implementation :

• The fee received here is 5e5.

expected(correct):

• The fee received is 5e17 .

@omega-audits can u please recheck the issue ?

@Ghoulouis

[omega-audits]

@batmanBinary , if you say that something is "expected" and "correct", that does not make it so , even if you repeat it several times. You need to argue why you think that is the case.

If I go more in detail in the issue, your argument seems to depend the assumption that there is something called "the pool's internal precision" (I am not sure what you mean by that) and the tokens "native precision" (I think you are referring to the decimals function of the ERC20 standard) Why do you believe this is relevant in the calculations here?