Wenwin contest - tnevler's results

Report

Low Issues

[L-1]: Incorrect comment

Context: /// @param rewardType 0 - staking reward, 1 - frontend reward. L72

Recommendation: https://github.com/code-423n4/2023-03-wenwin/blob/main/src/interfaces/ILottery.sol#L44 It should be like this: /// @param rewardType 0 - frontend reward, 1 - staking reward.

[L-2]: Add check it is not less than the minimum

Context:

1. if (_maxFailedAttempts > MAX_MAX_FAILED_ATTEMPTS) L27
2. if (_maxRequestDelay > MAX_REQUEST_DELAY L30

Recommendation: Check that _maxFailedAttempts and _maxRequestDelay is not too small.

[L-3]: Incorrect constant name

Context: uint256 private constant MAX_MAX_FAILED_ATTEMPTS = 10; L19

Recommendation: Change to uint256 private constant MAX_FAILED_ATTEMPTS = 10;

[L-4]: Invalid parameter type

Context:

1. modifier requireValidTicket(uint256 ticket) { L44 (Change to uint120. Here ticket is uint120)
2. uint256 ticket, L18 (Change to uint120)

Description: According TicketUtils.sol /// Ticket is represented as uint120 packed ticket:

[L-5]: Check if nonJackpotFixedRewards is correct

Context: nonJackpotFixedRewards = packFixedRewards(lotterySetupParams.fixedRewards); L91 Description: The user can make a mistake in the order of rewards. Recommendation: Check that each next reward is greater than the previous one.

[L-6]: Unsafe casting may overflow

Context:

1. uint16 reward = uint16(rewards[winTier] / divisor); L170
2. currentNetProfit += int256(unclaimedJackpotTickets * winAmount[drawId][selectionSize]); L275
3. unclaimedTickets[currentDraw][referrer].referrerTicketCount += uint128(numberOfTickets); L69
4. unclaimedTickets[currentDraw][player].playerTicketCount += uint128(numberOfTickets); L71
5. newProfit = oldProfit + int256(ticketsSalesToPot); L48
6. newProfit -= int256(expectedRewardsOut); L55
7. excessPotInt -= int256(fixedJackpotSize); L64
8. excessPot = excessPotInt > 0 ? uint256(excessPotInt) : 0; L65

Description: While Solidity 0.8.x checks for overflows on arithmetic operations, it does not do so for casting.

Recommendation: Use OpenZeppelin’s SafeCast library to prevent unexpected overflows.

Non-Critical Issues

[N-1]: Function defines a named return variable but then it uses return statements

Context:

1. return rewardPerTokenStored; L51
2. return rewardPerTokenStored + (unclaimedRewards * 1e18 / _totalSupply); L58
3. return balanceOf(account) * (rewardPerToken() - userRewardPerTokenPaid[account]) / 1e18 + rewards[account]; L62
4. return _baseJackpot(initialPot); L122
5. return 0; L124
6. return extracted * (10 ** (IERC20Metadata(address(rewardToken)).decimals() - 1)); L128
7. return drawRewardSize(currentDraw, winTier); L235
8. return LotteryMath.calculateReward( L239
9. return totalTicketsSoldPrevDraw.getPercentage(PercentageMath.ONE_PERCENT); L119
10. return totalTicketsSoldPrevDraw.getPercentage(PercentageMath.ONE_PERCENT * 75 / 100); L123
11. return totalTicketsSoldPrevDraw.getPercentage(PercentageMath.ONE_PERCENT * 50 / 100); L127
12. return playerRewardFirstDraw > decrease ? (playerRewardFirstDraw - decrease) : 0; L158
13. return rewardsToReferrersPerDraw[Math.min(rewardsToReferrersPerDraw.length - 1, drawId)]; L162
14. return number * percentage / PERCENTAGE_BASE; L18
15. return number * int256(percentage) / int256(PERCENTAGE_BASE); L23
16. return (ticketSize == selectionSize) && (ticket == uint256(0)); L32

Recommendation:

Choose named return variable or return statement. It is unnecessary to use both.

[N-2]: Follow Solidity standard naming conventions

Context:

1. function requestRandomnessFromUnderlyingSource() internal override returns (uint256 requestId) { L28 (requestRandomnessFromUnderlyingSource should start with_)
2. function fulfillRandomWords(uint256 requestId, uint256[] memory randomWords) internal override { L32 (fulfillRandomWords should start with_)
3. uint256 internal immutable firstDrawSchedule; L26 (firstDrawSchedule should start with_)
4. uint256 private immutable nonJackpotFixedRewards; L34 (nonJackpotFixedRewards should start with_)
5. uint256 private constant BASE_JACKPOT_PERCENTAGE = 30_030; // 30.03% L36 (BASE_JACKPOT_PERCENTAGE should start with_)
6. function packFixedRewards(uint256[] memory rewards) private view returns (uint256 packed) { L164 (packFixedRewards should start with_)
7. uint256 private claimedStakingRewardAtTicketId; L25 (claimedStakingRewardAtTicketId should start with_)
8. mapping(address => uint256) private frontendDueTicketSales; L26 (frontendDueTicketSales should start with_)
9. mapping(uint128 => mapping(uint120 => uint256)) private unclaimedCount; L27 (unclaimedCount should start with_)
10. function registerTicket( L181 (registerTicket should start with_)
11. function receiveRandomNumber(uint256 randomNumber) internal override onlyWhenExecutingDraw { L203 (receiveRandomNumber should start with_)
12. function drawRewardSize(uint128 drawId, uint8 winTier) private view returns (uint256 rewardSize) { L238 (drawRewardSize should start with_)
13. function dueTicketsSoldAndReset(address beneficiary) private returns (uint256 dueTickets) { L249 (dueTicketsSoldAndReset should start with_)
14. function claimWinningTicket(uint256 ticketId) private onlyTicketOwner(ticketId) returns (uint256 claimedAmount) { L259 (claimWinningTicket should start with_)
15. function returnUnclaimedJackpotToThePot() private { L271 (returnUnclaimedJackpotToThePot should start with_)
16. function requireFinishedDraw(uint128 drawId) internal view override { L279 (requireFinishedDraw should start with_)
17. function mintNativeTokens(address mintTo, uint256 amount) internal override { L285 (mintNativeTokens should start with_)
18. function markAsClaimed(uint256 ticketId) internal { L19 (markAsClaimed should start with_)
19. address internal immutable authorizedConsumer; L8 (authorizedConsumer should start with _)
20. mapping(uint256 => RandomnessRequest) internal requests; L9 (requests should start with _)
21. function fulfill(uint256 requestId, uint256 randomNumber) internal { L33 (fulfill should start with _)
22. function requestRandomnessFromUnderlyingSource() internal virtual returns (uint256 requestId); L48 (requestRandomnessFromUnderlyingSource should start with _)
23. uint256 private constant MAX_MAX_FAILED_ATTEMPTS = 10; L19 (MAX_MAX_FAILED_ATTEMPTS should start with_)
24. uint256 private constant MAX_REQUEST_DELAY = 5 hours; L20 (MAX_REQUEST_DELAY should start with_)
25. function requestRandomNumber() internal { L38 (requestRandomNumber should start with_)
26. function receiveRandomNumber(uint256 randomNumber) internal virtual; L58 (receiveRandomNumber should start with_)
27. function requestRandomNumberFromSource() private { L106 (requestRandomNumberFromSource should start with_)
28. function referralRegisterTickets( L52 (referralRegisterTickets should start with_)
29. function mintNativeTokens(address mintTo, uint256 amount) internal virtual; L74 (mintNativeTokens should start with_)
30. function referralDrawFinalize(uint128 drawFinalized, uint256 ticketsSoldDuringDraw) internal { L87 (referralDrawFinalize should start with_)
31. function getMinimumEligibleReferralsFactorCalculation(uint256 totalTicketsSoldPrevDraw) L111 (getMinimumEligibleReferralsFactorCalculation should start with_)
32. function requireFinishedDraw(uint128 drawId) internal view virtual; L134 (requireFinishedDraw should start with_)
33. function claimPerDraw(uint128 drawId) private returns (uint256 claimedReward) { L136 (claimPerDraw should start with_)
34. function playerRewardsPerDraw(uint128 drawId) internal view returns (uint256 rewards) { L156 (playerRewardsPerDraw should start with_)
35. function referrerRewardsPerDraw(uint128 drawId) internal view returns (uint256 rewards) { L161 (referrerRewardsPerDraw should start with_)

Description:

The above codes don't follow Solidity's standard naming convention.

• Internal and private functions should use mixedCase format starting with an underscore.
• Structs should be named using the CapWords style. Examples: MyCoin, Position, PositionXY.
• Events should be named using the CapWords style. Examples: Deposit, Transfer, Approval, BeforeTransfer, AfterTransfer.
• Functions should use mixedCase. Examples: getBalance, transfer, verifyOwner, addMember, changeOwner.
• Function arguments should use mixedCase. Examples: initialSupply, account, recipientAddress, senderAddress, newOwner.
• Local and State Variable should use mixedCase. Examples: totalSupply, remainingSupply, balancesOf, creatorAddress, isPreSale, tokenExchangeRate.
• Constants should be named with all capital letters with underscores separating words. Examples: MAX_BLOCKS, TOKEN_NAME, TOKEN_TICKER, CONTRACT_VERSION.
• Modifier should use mixedCase. Examples: onlyBy, onlyAfter, onlyDuringThePreSale.
• Enums, in the style of simple type declarations, should be named using the CapWords style. Examples: TokenGroup, Frame, HashStyle, CharacterLocation.

[N-3]: Wrong order of functions

Context:

1. modifier requireJackpotInitialized() { L104 (modifier definition can not go after constructor)
2. function onRandomNumberFulfilled(uint256 randomNumber) external override { L46 (external function can not go after intenal function)
3. function claimReferralReward(uint128[] memory drawIds) external override returns (uint256 claimedReward) { L76 (external function can not go after internal function)

Description:

According to official solidity documentation functions should be grouped according to their visibility and ordered:

• constructor

• receive function (if exists)

• fallback function (if exists)

• external

• public

• internal

• private

Within a grouping, place the view and pure functions last.

Recommendation:

Put the functions in the correct order according to the documentation.

[N-4]: add check >0 to avoid zero transfer

Context:

1. stakedToken.transferFrom(msg.sender, address(this), amount); L34 (check amount > 0)
2. stakedToken.transfer(msg.sender, amount); L47 (check amount > 0)
3. rewardToken.safeTransfer(beneficiary, claimedAmount); L156 (check claimedAmount > 0)
4. rewardToken.safeTransfer(msg.sender, claimedAmount); L175 (check claimedAmount > 0)