Native v2 (Security Review)

In the `CreditVault.sol` contract, the operations `settle()` and `removeCollateral()` are protected by the modifier `onlyTraderOrSettler`, which has the following code: ``` modifier onlyTraderOrSettler(address trader) { if (!isTraders[msg.sender] && !(isTraders[trader] && msg.sender == settlers[trader])) { revert ErrorsLib.OnlyTrader(); } _; } ``` We can observe that having `isTraders[msg.sender] == true` is enough to bypass this condition.

In `CreditVault.sol`, the storage variable `fundingFees` is used as an accounting variable for the funding fees accumulated when the function `epochUpdate()` is executed. However, the same variable is used to keep track of funding fees accumulated through different assets. This causes two problems (i) there is information loss on how much fee is available for each token, and (ii) there is a high risk of miscalculation due to the precision mismatch in the tokens. As an example, if `1e18 ETH` and `1e6 USDC` are accounted for in the contract as funding fees (with `fundingFees == 1e18 + 1e6`) that also owns `1e18 USDC`, it is possible for `feeWithdrawer` to withdraw `1e18 USDC` from the contract via `withdrawFundingFees()`. In addition, the address `feeWithdrawer` can withdraw any amount of underlying asset to any arbitrary recipient address via the function `withdrawFundingFees()`. However, there is no mechanism to make sure that the underlying token is supported by the contract (which could result in incorrectly reducing the value of `fundingFees`, resulting in the loss of these fees).

In the `CreditVault.sol` contract, anyone can call the function `addCollateral()` to add collateral for another address. However, there is no check to make sure that the receiver address is a whitelisted trader in the system. If this is not the case, tokens sent as collateral may remain locked in the contract.

`CreditVault.sol` contract has a function `setNativePool()` that is used to toggle the whitelist status of an instance of `NativeRFQPool.sol` contract. However, every call to the function, whether it's activating, de-activating, or no-op, would push the pool address in the function argument to the `poolArray`, which is the storage list used to keep track of the whitelisted addresses for the `NativeRFQPool.sol` instances. This does not seem aligned with the expected behavior.

The documentation and code comments indicate that the protocol is supposed to generate revenue by accumulating funding fees and also be able to distribute this to holders of LP tokens. The mechanisms by which this happens is not obvious from the code and in some cases the solvency of the fee mechanism is unclear. As an example, the mechanism by which LP Token gains fees is through `LPToken.distributeYield()` external call that is made on each `CreditVault.epochUpdate()` function call. However, the `distributeYield()` function simply updates the total underlying token amount of the LP token without actually transferring token. In addition, the auditors suspect that there may need to be some sort of invariant between `lpFee` and `fundingFee` such as `lpFee` < `fundingFee` because the LP fee actually originates from the funding fee accumulated. No such check takes place on-chain, and it is assumed that `accruedFees` input must be correct.

The following updates of the contract's state do not result in emitting an event, making it harder for observers (team or users) to track important contract state changes (intended or not) and react fast if something unexpected is identified: 1. Updates to privileged addresses in `CreditVault.sol` (`liquidator`, `signer`, `epochUpdated`, and `feeWithdrawer`); 2. Execution of the functions `NativeRouter.refundERC20()` and `NativeRouter.unwrapWETH9()`; 3. Adding routers to the whitelist in `NativeRouter.setWhitelistRouter()`;