Low

LiquidationPool::setPoolFeePercentage and LiquidationPool::constructor allows to set poolFeePercentage > HUNDRED_PC

To make the code more robust this constraint check should be added.

Use of tokens with blacklist or pause mechanism should be avoided

Some concreate issues have been reported as medium. However, as general rule tokens with blacklist or pause functionallities such as USDT, USDC and PAXG should be avoided. Reasons enumerated below:

• Blacklist of LiquidationPoolManager, LiquidationPoolManager.protocol will DOS liquidations

• Blacklist of a vault will DOS liquidations and asset recovery

• Blacklist of LiquidationPool will DOS rewards claims.

Same issues araise if pause mechanism are tirggered. If this tokens are suddenly blacklisted, then they will be also excluded as collateral from vaults, producing massive liquidations.

LiquidationPool consider adding a totalTST state variable

By doing this, all storage access when getTotalTST will be saved, The global variable should be increased/decreased whenever increasePosition/decreasePosition is called. Then getTotalTST can be deleted

Disclaimer: Using TST.balanceOf(this) is not an option, given that anyone can transfer TST to Liquidation pool, and next invariant would not hold:

$$Total ; TST = (\sum_{p}^{p ; \in ; Positions} (p.TST)) + (\sum_{ps}^{ps ; \in ; Pending ; stakes} (ps.TST)) $$

LiquidationPoolManager::forwardRemainingRewards blacklisted token can make whole function and dependencies revert

Some token like USDC and USDT have block lists. In case protocol address is blacklisted this function will always revert. This has enormous relevance for LiquidationPoolManager::runLiquidation given that it would DOS every vault liquidation which contain these tokens

Precedent: https://www.codehawks.com/report/cljyfxlc40003jq082s0wemya#M-03

Non-critical

Consider moving empty(positions[msg.sender]) from LiquidationPool::decreasePosition to LiquidationPool::deletePosition

Seems to be a better responsability distribution

LiquidationPool::decreasePosition does not follows CEI pattern

SmartVaultV3::liquidate allows to liquidate an already liquidated vault

Function does not check if the vault has been already liquidated

Gas

Use of Beacon Proxy pattern can save SmartVault deployment gas

Use of Beacon Proxy pattern can save ton of gas for new smart vault deployment. More

LiquidationPool::deleteHolder optimize function:

The function continue iterating even though holders was already removed.

LiquidationPool::consolidatePendingStakes and LiquidationPool::deletePendingStake can be optimeze to decrease considerable number of iterations

Whenever a holder increase his position, a pending stake is created, and it is push into pendingStakes, meaning that this new pending stake is put at the end of the array. This means that pendingStakes acts as a queue

When consolidatePendingStakes is called, it verifies if 24 hours has passed since each pending stake has been created, and each time it detects that a pending stake should be consolidated, it increase corresponding position TST and EUROS, and it shift the array to the right. To do this last shift, given solidity limitations each element is copied one position to the right. Therefore this last set of shifts is done once per position consolidated.

Given that we know that a pending stake that was pushed first has an earlier or equal deadline than a pending stake pushed after, we can simplify this set of shifts to a single one.

SmartVaultV3::getToken optimize loop

Apart from common optimization, return instruction can be added to avoid keep iterating inside the loop if the token was found:

Use unchecked{++i / --i} in for loops

Instance(23)

Cache length from storage/memory arrays when iterating over loops to save gas

Instance(23)

Do not intialize variable with default values

Instance(23)

Functions guaranteed to revert when called by normal users can be marked payable

If a function modifier such as onlyOwner is used, the function will revert if a normal user tries to pay the function. Marking the function as payable will lower the gas cost for legitimate callers because the compiler will not include checks for whether a payment was provided. The extra opcodes avoided are CALLVALUE(2), DUP1(3), ISZERO(3), PUSH2(3), JUMPI(10), PUSH1(3), DUP1(3), REVERT(0), JUMPDEST(1), POP(2), which costs an average of about 21 gas per call to the function, in addition to the extra deployment cost (2400 per instance).