cheatcode - Division Before Multiplication in Fixed-Point Arithmetic

[sherlock-admin]

cheatcode

medium

Division Before Multiplication in Fixed-Point Arithmetic

Summary

In the getCurrentSkew function of the FlatcoinVault contract, the division by 1e18 is performed after subtracting int256(sizeOpenedTotal) * unrecordedFunding. This could lead to a less precise calculation due to the division truncating the result.

Vulnerability Detail

Here's the relevant line of code:

return int256(sizeOpenedTotal) - int256(stableCollateralTotal) - (int256(sizeOpenedTotal) * unrecordedFunding) / 1e18;

In this line, the multiplication int256(sizeOpenedTotal) * unrecordedFunding is performed first, and then the result is divided by 1e18. The issue here is that division in Solidity (and in many other programming languages) is integer division, which means it rounds down to the nearest integer. This can lead to a loss of precision, especially when dealing with large numbers or numbers with many decimal places.

Impact

Precision loss in smart contracts can have significant implications. In the context of the FlatcoinVault contract, the getCurrentSkew function is used to calculate the current skew of the market, taking into account unaccrued funding. This skew is used in various calculations related to funding rates, fees, and other critical aspects of the contract.

If the skew is not calculated accurately due to precision loss, it could lead to inaccuracies in these subsequent calculations. This could potentially disadvantage certain parties or disrupt the intended functioning of the contract.

Code Snippet

https://github.com/sherlock-audit/2023-12-flatmoney/blob/main/flatcoin-v1/src/FlatcoinVault.sol#L324

Tool used

Manual Review

Recommendation

To minimize precision loss in fixed-point arithmetic, it's generally better to perform multiplication before division. This is because multiplication can increase the precision of the result, while division can decrease it.

In the case of the getCurrentSkew function, rearranging the operations so that the division by 1e18 is performed last could help to minimize precision loss. Here's how the revised line of code might look:

return int256(sizeOpenedTotal) - int256(stableCollateralTotal) - int256(sizeOpenedTotal * unrecordedFunding / 1e18);

In this revised line, the multiplication and division are performed together before the subtraction, which should help to maintain the precision of the calculation. However, it's important to note that this change could potentially introduce overflow issues, so appropriate safeguards should be put in place to prevent this.

[sherlock-admin]

1 comment(s) were left on this issue during the judging contest.

takarez commented:

invalid

[nevillehuang]

Invalid, lack of explicit example/PoC to prove impact of precision loss required by sherlock rules