<!-- canonical: https://0xsimao.com/findings/superfluid-locker-system-unlocking-multiplying-suffering-precision -->

# `FluidLocker::_getUnlockingPercentage()` divides before multiplying, suffering a significant precision error

Medium · Sherlock · Streaming · 20th November 2024

Finding M-2 of the Superfluid Locker System competition.

- Protocol: https://audits.sherlock.xyz/contests/648
- Report: /reports/superfluid-locker-system
- Codebase: https://github.com/0xsimao/2024-11-superfluid-locking-contract/tree/1fa5f86024be5f269e1a0898b1f939f1d4cce149
- Source: https://github.com/sherlock-audit/2024-11-superfluid-locking-contract-judging/issues/22

---

### Summary

[FluidLocker::_getUnlockingPercentage()](https://github.com/sherlock-audit/2024-11-superfluid-locking-contract/blob/main/fluid/packages/contracts/src/FluidLocker.sol#L384) is calculated as:
```solidity
function _getUnlockingPercentage(uint128 unlockPeriod) internal pure returns (uint256 unlockingPercentageBP) {
    unlockingPercentageBP = (
        _PERCENT_TO_BP
            * (
                ((80 * _SCALER) / Math.sqrt(540 * _SCALER)) * (Math.sqrt(unlockPeriod * _SCALER) / _SCALER)
                    + 20 * _SCALER
            )
    ) / _SCALER;
}
```
As can be seen, it divides before multiplying, leading to precision loss. 

The loss is always `((80 * 1e18) / Math.sqrt(540 * 24 * 3600 * 1e18)) = 1712139.4821`, so the 0.4821 component is discarded. This corresponds to `0.4821 * sqrt(540 * 24 * 3600 * 1e18) * 100 / 1e18 = 0.00032929935` BPS.

Note: this calculation assumed the other 2 issues are fixed.

As this loss is present in every calculation and it will make a 1 BPS different in many instances, it is significant. For example, if the maximum duration is picked, instead of `10000` BPS, it will actually be `9999 BPS` and 1 BPS goes to the TAX pool. If the user unlocks for example 1e5 USD, this is a 10 USD loss. As it will happen frequently, the loss will acumulate.


### Root Cause

In `FluidLocker::388`, it divides before multiplying.

### Internal pre-conditions

None.

### External pre-conditions

None.

### Attack Path

1. User calls unlock with vesting period, but due to the precision loss it rounds down and 1 BPS more goes to the tax distribution pool.

### Impact

User suffers a 1 BPS loss of funds. For example, 1e5 USD will yield a 10 USD loss.

### PoC

Presented in the summary.

### Mitigation

Multiply before dividing as it will never overflow.

---

Related findings:

- [Inconsistent Precision of Percentage Variables](https://0xsimao.com/findings/ostium-inconsistent-precision-percentage-variables): Ostium
- [Significant rounding errors due to `gameETH` not having precision](https://0xsimao.com/findings/codeup-rounding-game-eth-precision): CODEUP
- [Significant rounding error in whitelist discount that could be avoided](https://0xsimao.com/findings/1inch-rounding-whitelist-discount-avoided): 1inch Fee Extension
- [Pumponomics can be skipped when using FluidLocker::provideLiquidity](https://0xsimao.com/findings/superfluid-locker-system-ii-pumponomics-skipped-fluid-provide): Superfluid Locker Pumponomics