Inaccurate LP Token Supply Calculation Due to Convergence Issue

[howlbot-integration[bot]]

Lines of code

https://github.com/code-423n4/2024-07-basin/blob/main/src/functions/Stable2.sol#L74

Vulnerability details

Vulnerability Details

The calcLpTokenSupply function in the Stable2 contract is responsible for calculating the supply of liquidity provider tokens based on the reserves of the two underlying tokens. The function uses an iterative approach to converge on the correct LP token supply value.

It uses an absolute difference of 1 as the convergence criteria, checking if the difference between the current and previous lpTokenSupply values is less than or equal to 1. This approach can lead to the function returning a slightly inaccurate LP token supply value, especially when the actual value is close to an integer.

Code Snippet

for (uint256 i = 0; i < 255; i++) {
    uint256 dP = lpTokenSupply;
    dP = dP * lpTokenSupply / (scaledReserves[0] * N);
    dP = dP * lpTokenSupply / (scaledReserves[1] * N);
    uint256 prevReserves = lpTokenSupply;
    lpTokenSupply = (Ann * sumReserves / A_PRECISION + (dP * N)) * lpTokenSupply
        / (((Ann - A_PRECISION) * lpTokenSupply / A_PRECISION) + ((N + 1) * dP));
    // Equality with the precision of 1
    if (lpTokenSupply > prevReserves) {
        if (lpTokenSupply - prevReserves <= 1) return lpTokenSupply;
    } else {
        if (prevReserves - lpTokenSupply <= 1) return lpTokenSupply;
    }
}

Impact

The main impacts of the convergence issue in the calcLpTokenSupply function is on the accuracy of the calculated LP token supply and inaccurate accounting. When the actual LP token supply value is very close to an integer, the current convergence criteria of an absolute difference of 1 may not be strict enough, leading to the function returning a slightly inaccurate value.

The slightly inaccurate LP token supply could lead to suboptimal trading decisions or incorrect liquidity calculations, resulting in financial losses.

The inaccurate LP token supply could also lead to discrepancies in financial reporting, accounting, and other systems that rely on the accurate calculation of LP token balances.

Poc

function test_calcLpTokenSupply_convergence() public {
    _data = abi.encode(18, 18);
    uint256[] memory reserves = new uint256[](2);
    reserves[0] = 1e18;
    reserves[1] = 1e18;

    // Set the lpTokenSupply to a value very close to an integer
    uint256 expectedLpTokenSupply = 2_000_000_000_000_000_001;

    uint256 lpTokenSupply = _function.calcLpTokenSupply(reserves, _data);
    assertEq(lpTokenSupply, expectedLpTokenSupply, "Calculated LP token supply does not match expected value");
}

Run: forge test --match-test test_calcLpTokenSupply_convergence

Tools Used

Manual

Mitigation

Consider modifying the convergence criteria to use a smaller threshold, such as a relative difference of a small percentage instead of an absolute difference of 1. This would ensure that the function converges to a more accurate value of the LP token supply.

uint256 prevReserves = lpTokenSupply;
lpTokenSupply = (Ann * sumReserves / A_PRECISION + (dP * N)) * lpTokenSupply
    / (((Ann - A_PRECISION) * lpTokenSupply / A_PRECISION) + ((N + 1) * dP));

// Check if the relative difference is less than 0.01%
if (lpTokenSupply > prevReserves) {
    if (lpTokenSupply - prevReserves <= prevReserves * 1e-4) return lpTokenSupply;
} else {
    if (prevReserves - lpTokenSupply <= prevReserves * 1e-4) return lpTokenSupply;
}

Assessed type

Math

[Brean0]

An absolute value of 1 is the absolute lowest precision the contract can go, as prevReserves * 1e-4 would increase in absolute difference as the reserves scale up. Thus we believe the solution to be invalid. Given that the formulas used in the contract are not deterministic (i.e, require some form of estimation to calculate on chain), we feel to be an acceptable risk that the users of the protocol can take.

[alex-ppg]

The Warden outlines that the difference checked for an acceptable approximation is too high when dealing with integers and may result in a convergence succeeding even though it should fail. The recommended mitigation is invalid as Solidity does not support fractional values, and mandating an absolute convergence (i.e. a delta of 0) is ill-advised as it reduces the likelihood of an acceptable convergence being achieved.

I am inclined to agree with the Sponsor and maintain that it is acceptable to consider an approximation delta of 1 to be adequate irrespective of the actual reserve precision and value.

[c4-judge]

alex-ppg marked the issue as unsatisfactory: Invalid