UnchainX security record

The function `enqueue()` is designed to schedule an operation with a delay. However, it calls `_validationWithStaticCall()`, which makes the following sequence of static calls ```solidity for (uint256 i = 0; i < data.length; ++i) { (bool success, ) = data[i].target.staticcall(data[i].payload); if (!success) { revert InvalidData(data[i]); } } ``` Similarly `_execute()`, makes the same sequence of calls ```solidity for (uint256 i = 0; i < data.length; ++i) { (bool success, bytes memory returndata) = data[i].target.call(data[i].payload); Address.verifyCallResult(success, returndata); } ``` Both functions aim to execute the provided payloads; however, the static call in `_validationWithStaticCall()` will fail if any state changes occur. This prevents such transactions from being scheduled. Since most payloads should indeed change the blockchain state, this issue is given a *Critical* severity.

The logic of `totalRemainReward()` does not calculate the correct remaining reward due to the following errors in the logic. - The following logic divides the total rewards accumulated for a halving period and divides it by the total share, when it should give the total amount of rewards not the rewards per share. In addition, it should accumulate rewards for the entire halving period, which does not include the block of the halving as this is at the new reward rate. So that `tmpBlock` should be set to `halvingBlocks[i] - 1` and this logic should be executed if `lastBlock >= halvingBlocks[i]`. Not that the incremented amount should be careful to only accumulate rewards for blocks that should have a reward per block given by `rewardPerBlockOf(i)`. ```solidity if(lastBlock > halvingBlocks[i]) { // Calculate reward before halving remains += rewardPerBlockOf(i) * ((halvingBlocks[i] - 1) - tmpBlock + 1) * 1e18 / totalShare; tmpBlock = halvingBlocks[i]; } ``` - The following logic sets the return amount remains to the amount of rewards for since the last halving. However, it should increment by this amount so that it accounts for all rewards. In addition, the logic calls `rewardPerBlock()`, which calls `halvingProtocol.currentRewardPerBlock()` which returns 0 if the current block is greater than the end block. Thus if it is called after the `endBlock` it will add 0 to `remains` and not account for all of the rewards accumulated after the last halving. ```solidity if(tmpBlock <= lastBlock) { remains = rewardPerBlock() * (lastBlock - tmpBlock + 1); } ``` - The following line does not assign the result of the operation to `remains`. As such the amount returned will be the total amount of rewards as opposed to the total amount of remaining rewards. ```solidity remains - claimedReward; ```

The `TimeLockController` contract will not compile and will trigger the compiler error: ```text "Unimplemented feature (/solidity/libsolidity/codegen/ArrayUtils.cpp:227): Copying of type struct IBaseTimeLock.DataParams calldata[] calldata to storage not yet supported." ``` The issue stems from the following line: ```sol=54 _data[id] = params; ``` This line attempts to write a dynamic calldata array to a dynamic storage array, which is currently not supported.

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.

The function `preAllocateVotingPower()` checks if `preVotingPowerAllocation` is true to determine if the pre-allocation of voting power has been given. However, `preVotingPowerAllocation` is not set to true within the `preAllocateVotingPower()` function and there is no method in the contract to update it to true. In addition, `preAllocateVotingPower()` calls ` _updateVotes(params[i].account, 0, params[i].votingPower)` which assumes that users have no current votes. As such it should be ensured that this is called prior to `initialize()` being called otherwise if users have called `addShare()` before, then they will lose their votes obtained from burning tokens.

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.