0xloophole - Incorrect Order of Operations in _commitmentPrice Function

[sherlock-admin2]

0xloophole

Medium

Incorrect Order of Operations in _commitmentPrice Function

Summary

The ChainlinkFactory contract includes a _commitmentPrice function used to calculate the commitment price for Chainlink oracle updates. This function applies both a surcharge and a discount to the native quantity to determine the final fee.

Issue Details

The current implementation of the _commitmentPrice function applies the surcharge before the discount. This order of operations leads to higher fees compared to applying the discount first, potentially disadvantaging users.

Code Snippet

function _commitmentPrice(bytes32 underlyingId, uint256 nativeQuantity) internal view returns (uint256) {
    uint256 discount = feeManager.s_subscriberDiscounts(address(this), underlyingId, feeTokenAddress);
    uint256 surchargedFee = Math.ceilDiv(nativeQuantity * (PERCENTAGE_SCALAR + feeManager.s_nativeSurcharge()), PERCENTAGE_SCALAR);
    return Math.ceilDiv(surchargedFee * (PERCENTAGE_SCALAR - discount), PERCENTAGE_SCALAR);
}

Impact

1. Users are charged higher fees than necessary, especially when discounts are significant.
2. The fee calculation doesn't align with common pricing practices where discounts are typically applied to the base price before adding surcharges.
3. This may lead to user dissatisfaction and reduced competitiveness of the service.

Scenario

Consider the following scenario:

• Native Quantity: 1000
• Surcharge: 10% (0.1 * PERCENTAGE_SCALAR)
• Discount: 20% (0.2 * PERCENTAGE_SCALAR)

Current calculation:

1. Apply surcharge: 1000 * 1.1 = 1100
2. Apply discount: 1100 * 0.8 = 880

If we reverse the order:

1. Apply discount: 1000 * 0.8 = 800
2. Apply surcharge: 800 * 1.1 = 880

In this case, the result is the same, but for larger numbers and different discount/surcharge rates, the difference can be significant.

Proposed Fix

Reverse the order of operations in the _commitmentPrice function:

function _commitmentPrice(bytes32 underlyingId, uint256 nativeQuantity) internal view returns (uint256) {
    uint256 discount = feeManager.s_subscriberDiscounts(address(this), underlyingId, feeTokenAddress);
    uint256 discountedFee = Math.ceilDiv(nativeQuantity * (PERCENTAGE_SCALAR - discount), PERCENTAGE_SCALAR);
    return Math.ceilDiv(discountedFee * (PERCENTAGE_SCALAR + feeManager.s_nativeSurcharge()), PERCENTAGE_SCALAR);
}