Maker/taker will be paying slightly less funding fee to taker/maker respectively due to precision loss

Summary

Precision loss while calculating linear interpolation of points on curve due to division before multiplication.

Vulnerability Detail

The funding rate is used to determine the periodic funding payments between long and short positions. If there is a precision loss in the calculation, it can result in incorrect funding payments.

For calculating the jump rate / funding rate for the utilization rate inside JumpRateUtlizationCurve.sol file, its using the linear interpolation formula to find corresponding jump rate for the utilization rate. Below is the correct formula to calculate targetY (funding rate): $$targetY = startY + \frac{(targetX - startX)(endY- startY)}{(endX - startX)}$$

However, the formula used in the code to find it, is shown below, where division is occurring before multiplication which will results in precision loss. $$targetY = startY + \frac{(targetX - startX)}{(endX - startX)} * (endY- startY)$$

Due to the precision loss, maker/taker will be sending slightly less funding fee than they should be to taker/maker respectively as there is a precision loss while calculating funding rate. This means that the payments may not accurately reflect the intended redistribution of funds between maker/taker, leading to unfair outcomes or misaligned incentives.

Below is the vulnerable code: https://github.com/sherlock-audit/2023-05-perennial/blob/0f73469508a4cd3d90b382eac2112f012a5a9852/root/contracts/curve/CurveMath6.sol#L23-L36

function linearInterpolation(
    UFixed6 startX,
    Fixed6 startY,
    UFixed6 endX,
    Fixed6 endY,
    UFixed6 targetX
) internal pure returns (Fixed6) {
    if (targetX.lt(startX) || targetX.gt(endX)) revert CurveMath6OutOfBoundsError();

    UFixed6 xRange = endX.sub(startX);
    Fixed6 yRange = endY.sub(startY);
    UFixed6 xRatio = targetX.sub(startX).div(xRange);
    return yRange.mul(Fixed6Lib.from(xRatio)).add(startY); //@audit-issue Division before multiplication
}

Here, xRatio which is having a division, gets multiplied with yRange, which will results in precision loss while calculating jump rate.

Proof Of Concept

We wrote a PoC in foundry to demonstrate issue.

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;

import "forge-std/Test.sol";
import "root/number/types/UFixed6.sol";
import "root/number/types/Fixed6.sol";

contract PrecisionLossTest is Test {
    function setUp() public {}

    error CurveMath6OutOfBoundsError();
    // This is original one 
    function linearInterpolationWrong(
        UFixed6 startX,
        Fixed6 startY,
        UFixed6 endX,
        Fixed6 endY,
        UFixed6 targetX
    ) public pure returns (Fixed6) {
        if (targetX.lt(startX) || targetX.gt(endX)) revert CurveMath6OutOfBoundsError();

        UFixed6 xRange = endX.sub(startX);
        Fixed6 yRange = endY.sub(startY);
        UFixed6 xRatio = targetX.sub(startX).div(xRange);
        return yRange.mul(Fixed6Lib.from(xRatio)).add(startY); //@audit-issue Division before multiplication
    }

    // This is corrected
    function linearInterpolationCorrect(
        UFixed6 startX,
        Fixed6 startY,
        UFixed6 endX,
        Fixed6 endY,
        UFixed6 targetX
    ) public pure returns (Fixed6) {
        if (targetX.lt(startX) || targetX.gt(endX)) revert CurveMath6OutOfBoundsError();

        UFixed6 xRange = endX.sub(startX);
        Fixed6 yRange = endY.sub(startY);
        UFixed6 ratio = targetX.sub(startX).mul(UFixed6Lib.from(yRange)).div(xRange);
        return Fixed6Lib.from(ratio).add(startY);
    }

    function testLinearInterpolationPrecisionLoss() public pure {
        UFixed6 startX = UFixed6Lib.from(0);
        Fixed6 startY = Fixed6Lib.from(0);
        UFixed6 endX = UFixed6Lib.from(81); // targetUtilizationRate = 81%
        Fixed6 endY = Fixed6Lib.from(40); // targetFundingRate = 40%
        UFixed6 targetX = UFixed6Lib.from(57); // utilizationRate = 57%

        // Find funding rate corresponding to the provided utilization rate
        int cr = Fixed6.unwrap(linearInterpolationCorrect(startX, startY, endX, endY, targetX));
        int wr = Fixed6.unwrap(linearInterpolationWrong(startX, startY, endX, endY, targetX));

        console2.log("Correct targetY:", cr);
        console2.log("Wrong targetY  :", wr);

        assert(cr > wr);
    }
}

[PASS] testLinearInterpolationPrecisionLoss() (gas: 8182)
Logs:
  Correct targetY: 28148148
  Wrong targetY  : 28148120

As can be seen in output, the one written in actual code is producing slightly less funding rate than the correct one.

Impact

Precision loss while calculating linear interpolation of points on curve due to division before multiplication, which will results in wrong calculation of jump rate / funding rate. As a result will make taker/maker to pay slightly less funding fee to maker/taker respectively. This means that the payments may not accurately reflect the intended redistribution of funds between maker/taker, leading to unfair outcomes or misaligned incentives.

Code Snippet

https://github.com/sherlock-audit/2023-05-perennial/blob/0f73469508a4cd3d90b382eac2112f012a5a9852/root/contracts/curve/CurveMath6.sol#L23-L36 https://github.com/sherlock-audit/2023-05-perennial/blob/0f73469508a4cd3d90b382eac2112f012a5a9852/root/contracts/curve/CurveMath18.sol#L23-L36

Tool used

Manual Review, Foundry

Recommendation

We recommend to perform multiplication before division to avoid precision loss as shown in first formula above.

sherlock-audit / 2023-05-perennial-judging

BLACK-PANDA-REACH - Maker/taker will be paying slightly less funding fee to taker/maker respectively due to precision loss #208