Level Finance

Contracts inheriting from `Ownable` contain a function called `renounceOwnership()` which allows the `owner` to leave the contract without an `owner`. This would leave every function guarded by the `onlyOwner` modifier unable to be executed. In the case of `Erc20Reserve`, the `transfer` function, guarded by `onlyOwner`, is the only way of retrieving funds from the contract. If ownership is renounced in this context, the funds in the contract would be lost forever.

`LevelDevFund` inherits `Initializable`, a base contract used to "aid in writing upgradeable contracts, or any kind of contract that will be deployed behind a proxy". However, it is not written as an upgradeable contract and inherits from `Ownable`, which is not upgradeable. Therefore, when deployed, the contract will run the constructor from `Ownable`. At this point, it is possible for a malicious user to call `initialize`, setting `LEVEL_STAKE` to any desired value. Since the `owner` has no recourse to reset `LEVEL_STAKE`, the deployment would be a failure.

The `LyLevel` contract implements a token by attempting to follow ERC-20. However, the constant `decimals` is declared as `uint256` instead of `uint8` (see [EIP-20](https://eips.ethereum.org/EIPS/eip-20#token)). This is potentially a result of trying to clone-and-own the OpenZeppelin ERC20 code. This is generally considered poor practice, as it obfuscates the intentions of the developer, and neglects the benefits of using time-tested libraries such as OpenZeppelin's resulting in errors like the above.

When placing a swap order via `OrderManager` with a deflationary token, the `executeSwapOrder()` function attempts to use the same amount used when calling the `transfer()` function of the token. This will lead to the `OrderManager` attempting to swap more tokens than actually received during placement of the order. If there are not enough tokens in `OrderManager`, then the transaction will revert. Moreover, returning funds via `cancelOrder()` may fail due to insufficient token balance in `OrderManager`. In `LevelMasterV2`, `addLiquidity()` calculates ``` IERC20(assetToken).safeTransferFrom(msg.sender, address(this), assetAmount); IERC20(assetToken).safeIncreaseAllowance(address(levelPool), assetAmount); levelPool.addLiquidity(tranche, assetToken, assetAmount, minLpAmount, address(this)); ``` After the transfer from `msg.sender` to the `LevelMasterV2` contract, the actual transferred amount might be lower than the suppose `assetAmount`. This means we are potentially giving more allowance to the `levelPool` contract than it should have access to. However, this is not an issue in `levelPool.addLiquidity()` as it uses the actual balance changed through the call `_transferIn(_token, _amountIn)`.

When `addToken()` is called, the token's address is added to the `allAssets` array. However, there is no way for the protocol to remove added tokens, which means that once the `MAX_ASSETS` threshold is reached (currently set at 10 total assets), the protocol will not be able to add any new tokens or remove existing ones.

In `claimable()`, it is required that `_batchId <= currentBatchId`. However, it doesn't make sense to allow claiming at `currentBatchId` since `rewardPerShare` for the current batch would not have been set yet. This is only set in `allocateReward()`, but by calling it, `currentBatchId` will be incremented.

Division before multiplication may result in a loss of precision when the operations are carried over integer numbers. This was noticed in the following instances: * `LevelOracle` * #194 -- `lowerBound` * #195 -- `minLowerBound` * #196 -- `upperBound` * #197 -- `maxUpperBound` * `Pool` * #828 -- `tokenInfo.lastAccrualTimestamp` * #835 -- `tokenInfo.borrowIndex` * #836 -- `tokenInfo.lastAccrualTimestamp` * #888 -- `feeAdjust_scope_0`

`OrderManager` is currently not inheriting from the `IOrderManager` interface. It is generally recommended for Solidity contracts to inherit interfaces in order to define an abstract behavior for the extending contract. This helps to ensure that the contract implements the required functions and adheres to the specified behavior.

In `_reservePoolAsset()`, the `if` statement checks whether the asset's new reserved amount is greater than the pool amount. If this is not true, the logic is allowed to continue and `_reserveTrancheAsset()` is called. This check is being made because there should never be a situation where the pool amount is less than the reserved amount. However, `_reserveTrancheAsset()` increases not just the reserved amount, but also the pool amount. Therefore, the check is validating the wrong amounts. This may occasionally cause certain position increases to be blocked.

`setRiskFactor()` loops through an array of `RiskConfig` objects, one for each tranche with a risk factor to be modified. However, this leaves room for the user to mistakenly input a tranche twice, leading to an incorrect risk configuration.