[](https://hackmd.io/tdk5u4j5SOiRrCUSeB63yg)

## Medium severity issues

### M01. Excessive position exit
##### Description
In the `prepareReturn` functions, the balance of the `want` tokens is not taken into account when calculating the amount that needs to be withdrawn in the `withdrawSome` function. The contract may already be holding `want` tokens, which means a lesser amount can be withdrawn from the positions. This will increase the profit from the strategy.
#### Status : Fixed

#### Comment :

https://github.com/locus-finance/Vaults/pull/64) and (https://github.com/locus-finance/Vaults/pull/66)
Before calling `withdrawSome`, a check has been added to see if there are enough want tokens to cover the requested funds. If want tokens are not enough then `withdrawSome` is called with the missing amount as a parameter.

### M02. Already withdrawn want tokens are not accounted
##### Description
In the `liquidatePosition` function of the **FraxStrategy, LidoAuraStrategy, AuraWETHStrategy**, and **AuraBALStrategy** contracts, the balance of the `want` tokens is not taken into account when calculating the amount that needs to be withdrawn in the `withdrawSome` function. Currently, these contracts will not withdraw from the positions if the strategy has enough `want` tokens. However, if there are insufficient `want` tokens, the strategies will withdraw the entire `_amountNeeded` from the positions. It is possible to partially cover the `_amountNeeded` with the available `want` balance, which could potentially increase the strategy's profitability as fewer tokens are withdrawn from the positions.
#### Status : Fixed

#### Comment :
(https://github.com/locus-finance/Vaults/pull/84)
`withdrawSome` amount calculation updated to withdraw only necessary amount.

### M03. Code logic
##### Description
It is more efficient to sell the earned rewards first in the `_exitPosition` function of the **LidoAuraStrategy** and **RocketAuraStrategy** contracts. In addition to this, we recommend accounting these rewards as described in the M07 issue.
#### Status : Fixed
#### Comment :

[LidoAuraStrategy fix](https://github.com/locus-finance/Vaults/commit/01929aab007371015b50b2e0055cae016468e314) and [RocketAuraStrategy fix](https://github.com/locus-finance/Vaults/pull/72/files)
As recommended, we try to sell the earned rewards first.

### M04. Do not invest tokens during emergency exit
##### Description
We recommend checking for emergency exit status in the `adjustPosition` function before investing the tokens.
#### Status : Fixed

#### Comment :
(https://github.com/locus-finance/Vaults/pull/63)
Added `return` from `adjustPosition` in case of `emergencyExit` in each strategy

### M05. Inaccurate slippage calculation
##### Description
In the **CVXStrategy** contract, the `_expected` variable at line 332 is supposed to reflect the minimal expected swap output in `CRV`, but in reality, is calculated in `USDC` because of `cvxToWant`(`_cvxAmount`). The decimals value of `USDC` is 8, so there is no slippage protection as a result.
The same issue is present in the **FXSStrategy** contract at line 351.
#### Status : Fixed

#### Comment :

(https://github.com/locus-finance/Vaults/commit/87dd218e8e80d476c41ca14bfa574996ee360184)
Well noticed. We integrated CRV/CVX Uniswap V3 pool for gettig correct conversion rate.

### M06. Incorrect earned CVX calculations
##### Description
The `convertCrvToCvx` function of the **CVXRewardsMath** library calculates the reward in `CVX` tokens relative to `CRV` as the `mint` function of CVX token. The `convertCrvToCvx` function returns the amount after `if (cliff < totalCliffs)`. If `totalSupply` reaches the maximum, the whole amount is returned, whereas 0 should be returned.
#### Status : Fixed

#### Comment :

(https://github.com/locus-finance/Vaults/pull/73/files)
We had a look at Convex contracts and that is indeed how it needs to be.

### M07. Suboptimal withdrawal
##### Description
In the **FXSStrategy** contract, `_withdrawSome` function exits its Convex position so that the vault can collect the `_amountNeeded` of `want` tokens. Consider claiming the Convex rewards first to check if `_amountNeeded` could be covered by them so that the strategy would require to withdraw less tokens.
#### Status : Fixed

#### Comment :

(https://github.com/locus-finance/Vaults/commit/53880e2fbf1132184c5db8455cc9f16e74e99592)
As recommend, we try to cover the withdraw request with rewards first.

## Low severity issues

### L01. BAL token sell
##### Description
In the **AuraBALStrategy** contract, the `adjustPosition` function sells all `BAL` tokens at line 269. However, later in the code, this function proceeds to purchase `BAL` tokens. Consequently, the strategy incurs unnecessary losses by selling and rebuying tokens within the same transaction.
Similarly, the `prepareReturn` function also includes a `withdrawSome` call with the same sell logic at line 464 before invoking the `adjustPosition` function, leading to a similar loss of funds.
This issue is also present in the **AuraWETHStrategy** contract with the AURA tokens.
#### Status : Fixed

#### Comment :

(https://github.com/locus-finance/Vaults/pull/74/files)
Leads to more efficient tokens selling.

### L02. High slippage
##### Description
The project uses high slippage rates on swaps in some strategies (5%-20%). These swaps might be vulnerable to sandwich attacks.
#### Status : Fixed

#### Comment :
(https://github.com/locus-finance/Vaults/pull/81)
We try to use as low slippage rates as possible to make sure our strategies do not revert on swaps. All our strategies also have built-in functionality to change slippage rates in case there is a more efficient rate.

LidoAuraStrategy default slippage was decreased as well.

### L03. Code logic
##### Description
The `_sellBalAndAura` function of the LidoAuraStrategy contract checks if `(_balAmount == 0 || _auraAmount == 0)`, and will not sell any tokens if this statement is `true`. However, it is possible that `_auraAmount` is equal to 0, but `_balAmount` is not. In that case, no `_balAmount` will be sold.
#### Status : Fixed

#### Comment :
(https://github.com/locus-finance/Vaults/pull/67)
Added the ability to sell only BAL tokens if there are no AURA tokens.

### L04. Gas usage
##### Description
In `adjustPosition` function, the strategies perform a check to determine that the balance of `want` tokens is sufficient to cover `debtOutstanding` before doing any swaps of want tokens. However, if there is an insufficient balance to cover the `debtOutstanding`, it implies that no `want` will be swapped and hence no tokens could be deposited. As a result, it is possible to return from the function in case of insufficient balance in order to save gas on external balance check calls.
#### Status : Acknowledged

#### Comment :

If no `want` tokens were swapped, it is still possible that there are other tokens that could be used for opening a position. Therefore, we prefer to not return from the function even in the case when `want` balance is not enough to cover `debtOutstanding`. Balance checks on other tokens should be made first to ensure correct investment flow of a strategy.

### L05. Default params
##### Description
The `_pools` argument in the `exchange_multiple` function of the **CurveSwapRouter** is not required while calling. This argument already has a default value (array of zeroes). Thus, it can be omitted in the swaps.
#### Status : Fixed

#### Comment :
(https://github.com/locus-finance/Vaults/pull/68)
All calls to `multiple_exchange` fixed to not pass empty pools.

### L06. FXS rewards are not transferred upon migration
##### Description
`FXS` rewards should be withdrawn along with `CRV` и `CVX` tokens in the `prepareMigration` function of the FXSStrategy contract.
#### Status : Fixed

#### Comment :
(https://github.com/locus-finance/Vaults/pull/75)
As recommended, we now are sending FXS upon migration as well.

### L07. Gas costs
##### Description
All strategy contracts in the project request token decimals for every token conversion. We recommend either hardcoding them or storing them in `immutable` variables.
#### Status : Acknowledged

#### Comment :
(https://github.com/locus-finance/Vaults/pull/80)
We saved decimals to immutable variable in strategy constructor.

### L08. Hardcoded addresses
##### Description
Since curve booster addresses could theoretically change, we recommend adding functionality to update them.
#### Status : Acknowledged

#### Comment :
Since apparently changing the address of the booster is not a common scenario, we decided to leave the address hardcoded. If it changes, the strategy will be updated using the migration mechanism.

### L09. Inconsistent ways of price calculation
##### Description
The **LidoAuraStrategy** contract contains two methods for `BPT` price calculation: the `getBptPrice` function that uses oracle and the `IBalancerPool(bstethStable).getRate()` removing one of the methods or pointing that out in the documentation.
#### Status : Fixed

#### Comment :
(https://github.com/locus-finance/Vaults/pull/69)
Strategy was updated to use only TWAP oracle as a source for BPT price

### L10. Onchain price evaluation
##### Description
In **AuraWETHStrategy.sol**, the `getBptPrice` function uses onchain balances price evaluation. Balancer documentation notes that the contract should check for the vault reentrancy since this price formula might be manipulated. Despite the fact that we did not find any attacks on the current strategy, we still recommend checking for the reentrancy.
#### Status : Fixed

#### Comment :
(https://github.com/locus-finance/Vaults/pull/83)
Based on the recommendation in our chat with auditors, we integrated Balancer's solution to mitigate read-only reentrancy from their [VaultReentrancyLib](https://github.com/balancer/balancer-v2-monorepo/blob/9c1574b7d8c4f5040a016cdf79b9d2cc47364fd9/pkg/pool-utils/contracts/lib/VaultReentrancyLib.sol).

### L11. Redundant code
##### Description
The `adjustPosition` function of the FXSStrategy contract has unnecessary decimals scaling at lines 275-279. This can be simplified by using `FXS.decimals()` in the formula at line 274 instead of `want` token decimals. A similar case is in the **CVXStrategy** contract.
#### Status : Acknowledged

#### Comment :

Good point. However, using FXS.decimals() increases precision and this is not what we always want. In case when there is a very little amount of `want` tokens in strategy (i.e. dust), it is better to expect around 0 FXS in return. Increasing precision leads to expecting to receive a very small amount of FXS (0.00000000001 FXS, for example). This leads to UniswapV3 reverting with 'Too little received' when trying to get that small amount of tokens. We tried to implement the proposed change and this led to tests failing. Therefore, we prefer to leave that code.

### L12. Redundant code
##### Description
In the **FraxStrategy** contract, the `if` statement at line 110 could be removed since the contract will not have `frxEth` tokens on its balance. This is so because all `frxEth` tokens are sold just after being redeemed.
#### Status : Fixed

#### Comment :
(https://github.com/locus-finance/Vaults/pull/70)
Selling of frxETH is removed from `adjustPosition`

### L13. Redundant code
##### Description
In the **FraxStrategy** contract in `estimatedTotalAssets` and `prepareMigration` functions, the logic related to native ether could be removed. The contract will not be holding any native ether since all ether is deposited just after the `WETH.withdraw` call is done.
#### Status : Fixed

#### Comment :
(https://github.com/locus-finance/Vaults/pull/79)
Native eth was excluded from `estimatedTotalAssets` calculations in FraxStrategy. However transferring native eth in `prepareMigration` was left for rescue purposes.

### L14. Redundant code
##### Description
The AuraWETHStrategy contract contains redundant code in the `liquidateAllPositions` function. Specifically, lines 526-530 sell tokens and claim rewards, duplicating the code executed at the beginning of the `_exitPosition` function, which is called at line 531 in `liquidateAllPositions`.
#### Status : Fixed

#### Comment : (https://github.com/locus-finance/Vaults/pull/76)

### L15. Unused ERC20 returns
##### Description
The return values of ERC20 `approve` calls are not handled throughout the codebase in the project.
#### Status : Fixed

#### Comment :
(https://github.com/locus-finance/Vaults/pull/71)
We have replaced all calls to `approve` with `safeApprove` in which the success of the call is checked

### L16. Use of assert
##### Description
Line 319 in the LidoAuraStrategy contract contains an `assert`. We recommend changing it to a `revert` statement.
#### Status : Fixed

#### Comment :
(https://github.com/locus-finance/Vaults/pull/77)
`assert` was replaced with `require` instead of `revert` because there is no any complex logic expressions for usage of `revert`

### L17. Gas usage
##### Description
The `auraRewards()` depends on `balRewards()`, which calls `IConvexRewards(AURA_BASE_REWARD).earned()`. Through the codebase, there are places where `balRewards()` will be called twice (the first time to get `balRewards()` and second time, to calculate `auraRewards()`). We recommend saving the result of `balRewards()` and using it for calculation without calling the second time to save gas. The same issue is present in the strategies with `CRV` and `CVX` contracts.
#### Status : Fixed

#### Comment :
(https://github.com/locus-finance/Vaults/pull/85)
Now `balRewards()` value is cached and used as a parameter for `auraRewards()`

## Notes

### N01. Depeg of stablecoins
##### Description
In the **FXSStrategy* contract, if the price of the `FRAX` is lower than the price of `USDC`, the `fxsToWant` function returns more `USDC` tokens than expected. It can lead to the following:
- In the `_sellFxs` function, the `amountOutMinimum` parameter will increase. So it may happen that swap can revert since the allowable price change is very small. Although the effect might be mitigated because of TWAP used in `fxsToWant` function.
- In the `_adjustPosition` function, the slippage increases due to a decrease of `fxsExpectrdScaled` variable.
#### Status : Acknowledged

#### Comment :

### N02. Strategist reward that cannot be withdrawn
##### Description
The `harvest` function calls `vault.report` and it calls the `_assessFees` function, in which vault shares are sent to the strategy as a reward. They should generally belong to the owner of the strategy. However, the strategist will not be able to withdraw these tokens, and they get stuck in the strategy since there is no functionality to withdraw shares from the strategy.
#### Status : Acknowledged

#### Comment :
The concept of Locus Finance does not imply the ownership of a strategy by an outside actor like a strategist. So the strategist's reward will always be zero and will not be transferred to the strategy contract.

### N03. Tokens depeg
##### Description
In the case of the depeg of the `wstEth` and `rEth` tokens, there could be potential problems since the `bptPrice` is calculated related to the first token in the pool. In case the `bptPrice` is undervalued in the strategy, the strategy could lose tokens while exiting the pool due to slippage since the expected amount will be set lesser.
#### Status : Acknowledged

#### Comment :

### N04. Incorrect reward calculation
Due to logic in `AuraRewardsMAth.convertCrvToCvx` function, after the `inflationProtectionTime` time has passed, the `AURA` token rewards will be estimated as 0 (but in fact, there still could be rewards earned). This effects the `estimatedTotalAssets` function result, making this value lesser than it actually is.
#### Status : Acknowledged

#### Comment :
Due to the fact that the theoretical possibility of minting AURA rewards is present, it is currently unknown how this will be implemented in the future. The current inflation protection algorithm taken into account in the strategy.

#### Commits
Audit Commit `91a825efdcb1d49bc174e818a583dcd92a17fff3`

Final Commit `def6c51529b4b4119859039d02adaf8da62aa6c9`