UniBot (Diamond Protocol)

In `DiamondFactoryV3.stopLoss()` the fee to execute the operation for any position is fixed and is not a percentage of the position to be closed, meaning that a user with an amount lower or equal to the fee will not get any return value from his position being closed.

In the function `_removeUniLiquidity()`, the function `NonfungiblePositionManager.decreaseLiquidity()` is used. According to the documentation, it returns two parameters `amount0` and `amount1`, being respectively the amounts of `token0` and `token1` accounted for the position's tokens owed. However, these returned values are not checked. https://docs.uniswap.org/protocol/reference/periphery/NonfungiblePositionManager#decreaseliquidity Also, the function `NonfungiblePositionManager.collect()` is used. According to the documentation, the function `collect()` returns two parameters `amount0` and `amount1`, being respectively the amounts of fees collected in `token0` and `token1`. However, the code considers these returned values as the amounts of `token0` and `token1` removed from the pool. https://docs.uniswap.org/protocol/reference/periphery/NonfungiblePositionManager#collect

In the documentation (https://dmo-protocol.gitbook.io/diamond/unibot/pool), the formula to compute the Lending Rate includes a variable `Reserve Factor`. However, it does not appear in the function `getLendingRate()`.

The contract `DiamondFactoryController` uses an optional whitelisting mechanism to limit the addresses that can interact with some whitelisted functions. It is based on two data structures: - the state `mapping(address=>bool) contractWhiteList`; - a merkleTree with its root hash stored in the state variable `merkleRoot`; 1. One issue is that a MerkleTree data structure is used for access controls. Doing so has drawbacks: - for each update of the tree (address creation, update, removal), the variable `merkleRoot` must be updated in the contract; - the current tree state must be stored somewhere and maintained; - the list of authorized addresses is not visible on-chain; - an authorized address must know the current tree to build the parameter `_proof` needed to call a whitelisted function; 2. A second issue is that we can imagine an address being added to both data structures. Then, if the address needs to be removed, it should be removed from both data structures. If not, it would still be whitelisted in the second data structure.

As security standards develop, so does the Solidity language. To stay up to date with current practices, it's important to use a recent version of Solidity and conventions.

During the audit, the team shared their intent to use a proxy pattern for the contracts `DiamondFactoryV3` and `LendingPoolV3`. Adding a proxy contract that is not audited could add challenges and vulnerabilities to the project: - memory slot clashing; - selector clashing; - functions `init()` vulnerable to front-running; - any upgrade of the implementation contract should deal with the fact that the current business logic of `DiamondFactoryV3` is already close to 24576 bytes; - data should be stored in the proxy contract, not in the implementation contract;

Different Layer 2 networks can have different ways to handle the values of block.number and block.timestamp. As a result, vulnerabilities can appear: - For example, according to Uniswap V3 docs, the TWAP feature of a pool should be avoided on Optimism. https://docs.uniswap.org/protocol/concepts/V3-overview/oracle#oracles-integrations-on-layer-2-rollups - Also, according to Arbitrum docs, the rule of thumb is the following: "As a general rule, any timing assumptions a contract makes about block numbers and timestamps should be considered generally reliable in the longer term (i.e., on the order of at least several hours) but unreliable in the shorter term (minutes). (It so happens these are generally the same assumptions one should operate under when using block numbers directly on Ethereum!)" https://developer.arbitrum.io/time. The exact impact will depend on the chosen network, and it cannot be determined.