WATCHPUG - _accumulateFunding() maker will get the wrong amount of funding fee.

[sherlock-admin]

WATCHPUG

high

_accumulateFunding() maker will get the wrong amount of funding fee.

Summary

Vulnerability Detail

The formula that calculates the amount of funding in Version#_accumulateFunding() on the maker side is incorrect. This leads to an incorrect distribution of funding between the minor and the maker's side.

// Redirect net portion of minor's side to maker
if (fromPosition.long.gt(fromPosition.short)) {
    fundingValues.fundingMaker = fundingValues.fundingShort.mul(Fixed6Lib.from(fromPosition.skew().abs()));
    fundingValues.fundingShort = fundingValues.fundingShort.sub(fundingValues.fundingMaker);
}
if (fromPosition.short.gt(fromPosition.long)) {
    fundingValues.fundingMaker = fundingValues.fundingLong.mul(Fixed6Lib.from(fromPosition.skew().abs()));
    fundingValues.fundingLong = fundingValues.fundingLong.sub(fundingValues.fundingMaker);
}

PoC

Given:

• long/major: 1000
• short/minor: 1
• maker: 1

Then:

1. skew(): 999/1000
2. fundingMaker: 0.999 of the funding
3. fundingShort: 0.001 of the funding

While the maker only matches for 1 of the major part and contributes to half of the total short side, it takes the entire funding.

Impact

Code Snippet

https://github.com/sherlock-audit/2023-07-perennial/blob/main/perennial-v2/packages/perennial/contracts/types/Version.sol#L207-L215

Tool used

Manual Review

Recommendation

The correct formula to calculate the amount of funding belonging to the maker side should be:

fundingMakerRatio = min(maker, major - minor) / min(major, minor + maker)
fundingMaker = fundingMakerRatio * fundingMinor

[sherlock-admin]

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

141345 commented:

m

panprog commented:

medium because incorrect result only starts appearing if abs(long-short) > maker and the larger the difference, the more incorrect the split of funding is. But this situation is exceptional case, most of the time abs(long-short) < maker due to efficiency and liquidity limits

[arjun-io]

We'd like to re-open this as it does appear to be a valid issue. Medium severity seems correct here

[arjun-io]

Fixed: equilibria-xyz/perennial-v2#64

[MLON33]

From WatchPug,

Fixed alongside with #180.