Offchain orders cannot be filled correctly

Summary

If multiple offchain orders are being filled in a single transaction and one of them fails in _fillOrder function, all preceding successful orders in the same transaction will also be reverted.

Vulnerability Details

The current implementation of fillOffchainOrders can revert the entire transaction if the _fillOrder function call fails. Multiple orders may need to be resubmitted, increasing gas costs and transaction times.

https://github.com/Cyfrin/2024-07-zaros/blob/main/src/perpetuals/branches/SettlementBranch.sol#L297-L314

Here we can see the function call inside a loop (checking each order):

Now let's check _fillOrder function. In some scenarios it is possible that the trader does not meet the margin requirement check here:

Furthermore the _fillOrder function includes a call to revert Errors.InsufficientMargin to handle insufficient margin scenarios. However, this call cannot emit the correct order details, as it only provides the trading account ID and margin-related values, without specific details about the failed order. This prevents the backend servce to retry the correct order of the account.

Impact

• A trader places an off-chain order.

• The trader attempts to cancel all of his active orders and open a new position.

• _fillOrder is called and reverts due to the trader not meeting the margin requirements.

• The whole transaction reverts preventing further action on this order on the trader's end.

• All of the prevoius trader's orders remain active.

This leads to an order management issue where legitimate orders cannot be fulfilled or cancelled correctly.

Traders may experience potential financial loss due to the inability to manage their orders accurately.

Tools Used

Manual review

Recommendations

Implement try-catch blocks to handle _fillOrder calls individually. This approach allows each order to be processed independently, ensuring that a failure in one order does not revert the entire transaction.