New fee settings shouldn't be applied to already existing orders.

[code423n4]

Lines of code

https://github.com/code-423n4/2022-12-tigris/blob/b2ebb8ea1def4927a747e7a185174892506540ab/contracts/Trading.sol#L952
https://github.com/code-423n4/2022-12-tigris/blob/b2ebb8ea1def4927a747e7a185174892506540ab/contracts/Trading.sol#L493

Vulnerability details

Impact

New fee settings shouldn't be applied to already existing orders.

Otherwise, users might pay more fees than they've expected with the old fee settings.

Proof of Concept

Both OpenFees and CloseFees can be changed anytime by admin using setFees().

And when it calculates fees using _handleOpenFees() and _handleCloseFees(), it uses current fee settings and the below scenario would be possible.

1. At the first time, the open fee was 0.05% and a user opened a limit order using initiateLimitOrder() as the fee is low.
2. After that, the open fee was changed to 0.1% which is not good for a user.
3. When the pending order is executed using executeLimitOrder(), new fee(0.1%) will be used and it's not fair for the user.

Similar cases would happen with close fees as well.

Tools Used

Manual Review

Recommended Mitigation Steps

Recommend storing original fee settings when the order is opened and using those settings for the order.

[TriHaz]

Valid but we are not going to change it as fees will only be changed by DAO voting, so traders will have enough time to decide if they want to close with current fees.

[c4-sponsor]

TriHaz marked the issue as sponsor acknowledged

[c4-judge]

GalloDaSballo marked the issue as primary issue

[c4-judge]

GalloDaSballo marked the issue as duplicate of #377

[c4-judge]

GalloDaSballo marked the issue as satisfactory