Chain Staking

Function `emergencyWithdraw()` does not trigger the `_moveDelegates()` function like the `withdraw()` function does.

The function `getPriorVotes()` contains a `require()` statement that the input `blockNumber` is smaller than the current block number. ```solidity=310 function getPriorVotes(uint256 _pid, address account, uint blockNumber) public view returns (uint256) { require(blockNumber < block.number, "Comp::getPriorVotes: not yet determined"); ... ``` However, in the `propose()` function in the `CHNGovernance` contract, the following lines of code call the `getPriorVotes()` function with `blockNumber` set as `block.number`, which would cause the aforementioned `require()` statement to fail. ```solidity function getVotingPower(address user, uint blockNumber) public view returns (uint256) { return chnTokenStaking.getPriorVotes(poolId, user, blockNumber); } function propose(address[] memory targets, uint[] memory values, string[] memory signatures, bytes[] memory calldatas, string memory description) public returns (uint) { require(getVotingPower(msg.sender, block.number) >= proposalThreshold, "GovernorAlpha::propose: proposer votes below proposal threshold"); ... ```

In the `add()` and `set()` functions, the flag `_withUpdate` determines if all the pools will be updated. This reliance might lead to significant loss of the reward. For illustration, assume we have only one pool with `pool.allocPoint == 50` and `totalAllocPoint == 50` at the beginning. Now we want to add another pool with `pool.allocPoint == 50`. There will be two scenarios on calculating the pool reward, Case 1: `_withUpdate` is set to `true`. - distribute the reward and update the pool. - add the given pool information Case 2: `_withUpdate` is set to `false`. - add the given pool information (Note: While we focused on the `add()` function, both the `add()` and `set()` functions update `totalAllocPoint`, which is used in calculation of pool rewards in the function `updatePool()` * In Case 1, reward for the first pool is updated in the call to `updatePool()` where `reward = multiplier.mul(rewardPerBlock).mul(pool.allocPoint).div(totalAllocPoint)`. * In Case 2, an update `totalAllocPoint = totalAllocPoint.add(_allocPoint)` is done first. Then `updatePool()` calculates the reward for the first pool: `reward = multiplier.mul(rewardPerBlock).mul(pool.allocPoint).div(totalAllocPoint);` Because the second pool is sharing rewards with the first one, the amount of reward for the first pool becomes half as much as that in the first case.

The comments above the `add()` function express the documented constraint that the same stake token should not be added more than once with the `add()` function. The current implementation is relying on the `owner` to avoid repeatedly adding the same stake token to the pool, as the function will only be called by the owner."

```solidity=80 require(_rewardPerBlock < _rewardToken.totalSupply(), "Reward per block less than token total supply"); ``` The error message above is incorrect.

There are no checks to validate if a `_pid` exists in the following functions: - `set()` - `pendingReward()` - `updatePool()` - `stake()` - `withdraw()` - `emergencyWithdraw()` - `claimRewardFromVault()`

```solidity=112 function getMultiplier(uint256 _from, uint256 _to) public view returns (uint256) { if (_to <= bonusEndBlock) { return _to.sub(_from).mul(BONUS_MULTIPLIER); } else if (_from >= bonusEndBlock) { return _to.sub(_from); } else { return bonusEndBlock.sub(_from).mul(BONUS_MULTIPLIER).add( _to.sub(bonusEndBlock) ); } } ``` Function call of `getMultiplier()` may return a wrong result if inputted block number, `_from`, is smaller than `startBlock`. This does NOT affect the core functions because they only use block numbers that are bigger than `startBlock` when calling function `getMultiplier()`. But wrong return values of function `getMultiplier()` may still misguide users.