sherlock-audit / 2024-01-napier-judging

9 stars 6 forks source link

cheatcode - Rounding Errors During Fee Deduction leads to less fees being collected #30

Closed sherlock-admin2 closed 7 months ago

sherlock-admin2 commented 7 months ago

cheatcode

medium

Rounding Errors During Fee Deduction leads to less fees being collected

Summary

There is inaccurate issuance fee accounting in the issue function due to rounding errors.

uint256 sharesUsed = sharesMinted + accruedInTarget;
        uint256 fee = sharesUsed.mulDivUp(issuanceFeeBps, MAX_BPS);
        issued = (sharesUsed - fee).mulWadDown(_maxscale);

        // Accumulate issueance fee in units of target token
        issuanceFees += fee;

Vulnerability Detail

When subtracting the fee from sharesUsed to calculate issued, there is an implicit decimal conversion happening.

sharesUsed = 100.5
fee = 0.3

If we simply subtract:
issued = sharesUsed - fee 
issued = 100.5 - 0.3 = 100.2

However, after the decimal conversion for the tokens' precision level, the result may look like:

sharesUsed = 100
fee = 0
issued = 100

So the fee gets rounded down to 0 during the decimal conversion. This means no fees were actually deducted from the issued amount.

Over time, these small rounding errors during decimal conversions could compound and lead to less fees being collected than expected.

Impact

Reduced critical revenue for the protocol and fee recipients.

Code Snippet

https://github.com/sherlock-audit/2024-01-napier/blob/main/napier-v1/src/Tranche.sol#L218C9-L220C1

Tool used

Manual Review

Recommendation

Use OpenZeppelin's SignedSafeMath for subtracting fees to prevent rounding risk:


using SignedSafeMath for int256;

int256 sharesUsedSigned = sharesUsed.toInt256();

int256 issuedSigned = sharesUsedSigned - fee; 

uint256 issued = issuedSigned.toUint256();
sherlock-admin commented 7 months ago

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

tsvetanovv commented:

Low