joestakey - Incorrect funding amount due to precision loss in `getNextFundingAmountPerSize()` for markets with low open interest
Closed this issue · 0 comments
joestakey
medium
Incorrect funding amount due to precision loss in getNextFundingAmountPerSize()
for markets with low open interest
Summary
Rounding happens in getNextFundingAmountPerSize()
due to division before mulitplication, leading to a loss of precision impacting the funding amount.
Vulnerability Detail
cache.fundingUsd
is computed by first dividing cache.sizeOfLargerSide
by Precision.FLOAT_PRECISION == 1e30
.
The issue is that cache.sizeOfLargerSide
also has 30 decimals, meaning that if the open interests are low enough, the rounding will be significant.
For instance: cache.oi.longOpenInterest = 1.99999e30
-> cache.sizeOfLargerSide / Precision.FLOAT_PRECISION = 1
.
File: contracts/market/MarketUtils.sol
941: cache.sizeOfLargerSide = cache.oi.longOpenInterest > cache.oi.shortOpenInterest ? cache.oi.longOpenInterest : cache.oi.shortOpenInterest;
942: result.fundingFactorPerSecond = getFundingFactorPerSecond(
943: dataStore,
944: market.marketToken,
945: cache.diffUsd,
946: cache.totalOpenInterest
947: );
948: cache.fundingUsd = (cache.sizeOfLargerSide / Precision.FLOAT_PRECISION) * cache.durationInSeconds * result.fundingFactorPerSecond;//@audit precision loss
Impact
Precision loss results in a lower funding amount, which means the position fees computed in PositionPricingUtils.getPositionFees()
will be incorrect.
Code Snippet
Tool used
Manual Review
Recommendation
Refactor so that division happens after multiplication.
Overflow could technically occur for very large open interests, but this is expected as per the comments in getTotalBorrowing()
.
+948: cache.fundingUsd = cache.sizeOfLargerSide * cache.durationInSeconds * result.fundingFactorPerSecond / Precision.FLOAT_PRECISION;
-948: cache.fundingUsd = (cache.sizeOfLargerSide / Precision.FLOAT_PRECISION) * cache.durationInSeconds * result.fundingFactorPerSecond;
Duplicate of #113