rvierdiiev - fundingFee should be updated in same way as makerFee, takeFee, positioFee

[sherlock-admin]

rvierdiiev

medium

fundingFee should be updated in same way as makerFee, takeFee, positioFee

Summary

fundingFee can be changed by product coordinator. It uses settleProduct modifier, which calls settle function for a product that will settle product as latestVersion -> settleVersion -> currentVersion. While makerFee, takeFee, positioFee can be changed only when previous position is settled. They are changed like latestVersion -> settleVersion -> fee change -> currentVersion.

Vulnerability Detail

Inside settle function there is _settleFeeUpdates call, which is done just after previous period is settled. This is done to not affect previous traders, that agreed on previous fees, when they did trade. So when transition is done from latestVersion to settleVersion, then this fees are applied. And only then transition is done from settleVersion to currentVersion which already uses new fees.

So when owner wants to change takerFee for example, then in case if position is not settled yet, then this update will be queued.

However, there is one more fee, that coordinator can provide and that can affect traders. It's fundingFee and it can be updated only after settle function. That means that transition will be done from latestVersion to settleVersion and to currentVersion adn only then fees will be applied.

This is incorrect and should be done in same way as makerFee, takeFee, positioFee.

Impact

Fees are changed later then needed

Code Snippet

Provided above

Tool used

Manual Review

Recommendation

Change fundingFee in same way as makerFee, takeFee, positioFee. In case if pre is not empty, then queue update.

[arjun-io]

The reason the other fees require a settlement is due to account issues that would arise if settlement did not happen - because the fee is charged on pre-positions rather than settled positions. In the case of funding, there is not accounting issues that would arise if settlement happens or not, because funding is charged on settled positions.

[KenzoAgada]

Closing issue due to sponsor comment and low impact.