QA Report

[code423n4]

• The error message should be <= 0 :

  require(_maintenanceMargin > 0, "_maintenanceMargin < 0");

• There are functions that do not follow the Check-Effects-Interaction pattern, e.g. addMarginFor, processWithdrawals. They have external calls in the middle of execution, e.g. inside the loop, so should have extra protection from re-entrancy just in case, unless you 100% trust these external contracts (e.g. tokens), but nevertheless I think you should always act preventively.

• VUSD returns hardcoded 6 decimals:

  function decimals() public pure override returns (uint8) {
      return 6;
  }

  While in practice it should be tied with USDC token that has 6 decimals:

  /// @notice vUSD is backed 1:1 with reserveToken (USDC)
  IERC20 public immutable reserveToken;

  There is no restriction of setting another reserveToken, so consider calling .decimals() when setting the reserveToken, and then assign the same value to the VUSD decimals.

• There are TODOs left:

  // @todo put checks on slippage
  // sending market orders can fk the trader. @todo put some safe guards around price of liquidations
  // @todo handle case when totalPosition = 0
  @todo consider providing some incentive from insurance fund to execute a liquidation in this scenario.

• I think the condition should not be inclusive here:

  while (i < withdrawals.length && (i - start) <= maxWithdrawalProcesses)

  e.g. when maxWithdrawalProcesses = 3, it will actually execute the loop 4 times.

• Consider introducing reasonable upper and lower limits for setMaxWithdrawalProcesses, otherwise an admin can grief by setting it to 0 and thus blocking the withdrawals, unless this may be intended.

• Oracle always assumes that the result will have 8 decimals, thus it divides by a hardcoded value of 100. You should verify that by calling .decimals on Chainlink oracle: https://docs.chain.link/docs/price-feeds-api-reference/#decimals

[atvanguard]

The last issue is a duplicate of #21. The rest are acknowledged.