Unchain-X

Functions `rewardPerBlockOf()` and `currentRewardPerBlock()` use the following to calculate a reward per block amount: ```solidity reward = initReward / (2 ** halvingNum) / 28800 ``` where `halvingNum` may be replaced by `totalNum` within function `rewardPerBlockOf()` in the case where `halvingNum > totalNum`. This calculation is incorrect for how the `endBlock` is set up in the `constructor`: ```solidity endBlock = options.genesisBlock + (options.halvingInterval * options.totalNum) + ((options.totalSupply - totalMiningBeforeLastHalving) / (options.initReward / (2 ** options.totalNum))) - 1; ``` `endBlock` is composed of the taking the `genesisBlock` and adding the `totalNum` multiple of the `halvingInterval` in order to get to the last halving block; This value is summed with the difference in the total supply and the total mined before the last halving block, then divided by what is assumed to be the reward per block in order to get the number of blocks to be added to the last halving block in order to obtain the `endBlock`. In other words, the rewards per block are assumed to be `initReward / (2 ** halvingNum)` where there is no division by `28800`.

The functions `activate()` and `deactivate()` can be called multiple times via the functions `list()` and `delist()` in the contract `UNXWapV3LmFactory`. However, the logic assumes that if a pool is deactivated, that it will not be activated again. This is because when `deactivate()` is called, it sets `lastActivedBlock` to be the current block number and `actived` to false. Then the function `accumulateReward()` will treat the `lastActivedBlock` as the `endBlock` as opposed to the end block of the halving protocol. The logic will then only accumulate rewards up until the `lastActivedBlock`, and after it is accumulated to this point the function `accumulateReward()` will simply return without updating the `lastUpdateBlock`. Then if `activate()` is called after a pool was deactivated, it will update `actived` to be true, so that it will have the halving end block as the `endBlock`. This will then allow rewards to be accumulated from the `lastUpdateBlock` to the halving end block, which can include time that the pool was deactivated and should not accumulate rewards. As this can allocate funds that are not accounted for, it may cause the `HalvingProtocol` to run out of funds, resulting in users being unable to harvest rewards they are entitled to. In addition, it can cause users to be able to claim additional rewards that they should not be entitled to.

If `deactivate()` is called after the `halvingProtocol.endBlock()`, then it will update `lastActivedBlock` to a block number greater than the `halvingProtocol.endBlock()`. When `accumulateReward()` is called, it will then take the `endBlock`, which is the block to which to accumulate rewards, as the `lastActivatedBlock`. This will accumulate rewards for a larger period than expected and may cause the `HalvingProtocol` to run out of funds, resulting in users being unable to harvest rewards they are entitled to. In addition, it can cause users to be able to claim additional rewards that they should not be entitled to.

The function `allocate()` in the UNXwapV3Manager allows the owner or executor to set an allocation amount for lmPools. This does so by calling `allocate()` in the `UNXwapV3LmFactory`, which performs checks to ensure the total allocation does not exceed the max allocation and calls `setAllocation()` on the individual lmPools. The function `setAllocation` in the `UNXwapV3LmPool` first calls `accumulateReward()` and then sets the new allocation amount. However, if `accumulateReward()` is called when the pool is deactivated, then it will not update the `lastUpdateBlock`. If it is called after the genesis block when the protocol is deactivated it can allow for unintended rewards to be allocated if the protocol is then activated.

When a `v3Pool` is created, an associated `lmPool` is created in the function `createPool()`. This `lmPool` is then synced with the state of `v3Pool` and has its liquidity updated via the `NonfungiblePositionManager` when there are updates to the `v3Pool` liquidity. If the function `setLmPool()` is used to change the `lmPool` to another address, then the `lmPool` will no longer be synced with the `v3Pool` or the `NonfungiblePositionManager`. This can make it so that users do not accumulate rewards when they should or accumulate more rewards than they should. In addition, in the `UNXwapV3LmFactory` when an `lmPool` is created it is set in the mapping `lmPools` which is used in the check for `transferReward()` and when listing, delisting, or allocating rewards for a `v3Pool`. The mapping `lmPools` is only set when creating a pool, so that if `setLmPool()` is called these will no longer match.

The function `setMaxAllocation()` does not check if the input `maxValue` is less than the current `totalAllocation`. Thus it is possible that `maxAllocation` is less than the current `totalAllocation`. The `HalvingProtocol` has the ability to have multiple operators to allow the rewarding of the stored `token`. If these operators determine the amount they can allocate by utilizing the `maxAllocation` variable, then this can cause an overallocation of funds and prevent some users from collecting rewards they are entitled to.

The function `_updateLmPosition()` is called last during liquidity operations to call the lmPool to sync its liquidity with that of the v3Pool. However, it is possible to reenter the protocol to execute operations before the v3Pool and its associated lmPool have their liquidity values synced. In particular, if tokens with hooks are supported, during the transfer, they may make an external call allowing users to reenter the protocol. As this happens before the call to `_updateLmPostion()` it allows users to interact with the protocol when the v3Pool and lmPool are not fully synced. While no explicit attack vector was found, due to the time constraints of the audit we were not able to consider all possibilities. Considering the reentrancy allows interaction with the protocol when the lmPool is not fully updated, it could cause rewards to be allocated unexpectedly so we give this a medium severity.

The lines cited above set variables to their default value. The keyword `delete` can be used in their place as it more closely matches the semantics of the logic.

The increment operation saves a small amount of gas if performed from the left (`++i`) instead of from the righthand side (`i++`).

The logic of `accumulateReward()` will not allocate rewards for the genesis block as it assumes it is accumulating starting from the genesis block.