Megamoon - Audit

As the implementation of function `_updateUserProfit`, the users' reward and latest balance are updated in the lotteryId range between `user.lotteryId` and `currentLotteryId`. ```solidity=511 uint256 userId = user.lotteryId; if (user.lockedAmount > 0) { _userInjectionHistory[msg.sender][userId] = user.lockedAmount; _userRewardHistory[msg.sender][userId] = user.lockedAmount * lotteryInjectionInfos[userId].returnAmount / lotteryInjectionInfos[userId].amount; latestBalance += _userRewardHistory[msg.sender][userId]; userId += 1; } for (userId; userId < currentLotteryId; userId++) { uint256 injectAmount = latestBalance * lotteryInjectionInfos[userId].injectPercentage / 10000; _userInjectionHistory[msg.sender][userId] = injectAmount; uint256 rewardAmount = injectAmount * lotteryInjectionInfos[userId].returnAmount / lotteryInjectionInfos[userId].amount; _userRewardHistory[msg.sender][userId] = rewardAmount; latestBalance -= injectAmount; latestBalance += rewardAmount; } if (currentLotteryId == nextLotteryId || currentLotteryId == 0) { // lottery close user.lockedAmount = 0; user.stakingAmount = latestBalance; } else { user.lockedAmount = latestBalance * lotteryInjectionInfos[userId].injectPercentage / 10000; user.stakingAmount = latestBalance - user.lockedAmount; } user.lotteryId = userId; // currentLotteryId ``` But the `currentLotteryId` could be reset in the function `setLotteryAddress`. When the currentLotteryId is reset from `originLotteryId` to `newLotteryId`, the user whose lotteryId > `newLotteryId` will lose their rewards from `user.lotteryId` to the `originLotteryId`.

The function `setActiveOrInactive` do not check if the user is registered before update `isActive[msg.sender]`, and the function is open to the public. Invalid user data could be set in the contracts by the attackers,

Winthin function `returnFunds`, the input `_lotteryId` is used for emit event `ReturnFunds`. But the actual return fund logic uses the statement `currentLotteryId`.

When the same LP token is added into a pool more than once in function `add()`, the total amount of reward in function `updatePool()` will be incorrectly calculated. The current implementation is relying on the operation correctness to avoid repeatedly adding the same LP token to the pool, as the function will only be called by the owner.

The order of external call/transfer and storage manipulation should follow the check-effect-interaction pattern.

The Lottery Index used in `latestBalance` Calculation is named `userId` which is not named as how it used. ```solidity= uint256 userId = user.lotteryId; ```

Modifier notContract is not used in contracts. ```solidity=112 modifier notContract() { require(!_isContract(msg.sender), "contract not allowed"); require(msg.sender == tx.origin, "proxy contract not allowed"); _; } ```

The inconsistent specification in comments: ```solidity=79 uint256 public constant MAX_PROCESSING_FEE = 10000; // 500 = 5% uint256 public constant MAX_WITHDRAW_FEE = 10000; // 500 = 5% uint256 public constant MAX_WITHDRAW_FEE_PERIOD = 744 hours; // 14 days (1 hour = 3600 blocks) ```

One or more state changes do not emit events to pass the changes out of chain. ```solidity=207 lottery = IMegamoonLottery(lotteryAddress); currentLotteryId = lottery.viewCurrentLotteryId(); nextLotteryId = 1; ``` --- ```solidity=244 processingFee = _performanceFee; ``` --- ```solidity=253 withdrawFee = _withdrawFee; ``` --- ```solidity=265 withdrawFeePeriod = _withdrawFeePeriod; ``` --- ```solidity=275 minPriceTicketInMoon = _minPriceTicketInMoon; maxPriceTicketInMoon = _maxPriceTicketInMoon; ``` --- ```solidity=285 maxNumberTicketsPerBuyOrClaim = _maxNumberTicketsPerBuy; ``` --- ```solidity=295 referralBonus = _referralBonus; ``` --- ```solidity=679 BONUS_MULTIPLIER = multiplierNumber; ```

In the `add()` and `set()` functions, the flag '_withUpdate' determines if all the pools will be updated. This reliance might lead to a 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, the `updateAllocPoint()` and `set()` functions also 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 `neuronReward =multiplier.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: `neuronReward =multiplier.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.