Wenwin contest - seeu's results

Low issues

[L-01] Outdated Compiler Version

Description

Using an older compiler version might be risky, especially if the version in question has faults and problems that have been made public.

Findings

• src/VRFv2RNSource.sol => ^0.8.7
• src/interfaces/IVRFv2RNSource.sol => ^0.8.7

References

• SWC-102
• Etherscan Solidity Bug Info

[L-02] Compiler version Pragma is non-specific

Description

For non-library contracts, floating pragmas may be a security risk for application implementations, since a known vulnerable compiler version may accidentally be selected or security tools might fallback to an older compiler version ending up checking a different EVM compilation that is ultimately deployed on the blockchain.

Findings

• src/VRFv2RNSource.sol => pragma solidity ^0.8.7;
• src/staking/StakedTokenLock.sol => pragma solidity ^0.8.17;

References

• L003 - Unspecific Compiler Version Pragma
• Version Pragma | Solidity documents
• 4.6 Unspecific compiler version pragma | Consensys Audit of 1inch Liquidity Protocol

[L-03] Use _safeMint instead of _mint

Description

In favor of _safeMint(), which guarantees that the receiver is either an EOA or implements IERC721Receiver, _mint() is deprecated.

Findings

• src/LotteryToken.sol

  ::19 =>         _mint(msg.sender, INITIAL_SUPPLY);
  ::26 =>         _mint(account, amount);

• src/Ticket.sol

  ::26 =>         _mint(to, ticketId);

• src/staking/Staking.sol

  ::74 =>         _mint(msg.sender, amount);

References

• OpenZeppelin warning ERC721.sol#L271
• solmate _safeMint
• OpenZeppelin _safeMint

[L-04] Use require instead of assert

Description

It is reccomended to use require instead of assert since the latest, when false, uses up all the remaining gas and reverts all the changes made.

Findings

• src/LotterySetup.sol#L147

  assert(initialPot > 0);

• src/TicketUtils.sol#L99

  assert((winTier <= selectionSize) && (intersection == uint256(0)));

References

• Require vs Assert in Solidity

[L-05] decimals() is not part of ERC20 standard and it may fail

Description

Since decimals() is not a part of the official ERC20 standard, it could not work for some tokens.

Findings

• src/LotterySetup.sol

  ::79 =>         uint256 tokenUnit = 10 ** IERC20Metadata(address(lotterySetupParams.token)).decimals();
  ::128 =>             return extracted * (10 ** (IERC20Metadata(address(rewardToken)).decimals() - 1));
  ::168 =>         uint256 divisor = 10 ** (IERC20Metadata(address(rewardToken)).decimals() - 1);

References

• [L-02] DECIMALS() NOT PART OF ERC20 STANDARD

[L-06] Timestamp dependency

Description

The timestamp of a block is provided by the miner who mined the block. As a result, the timestamp is not guaranteed to be accurate or to be the same across different nodes in the network. In particular, an attacker can potentially mine a block with a timestamp that is favorable to them, known as "selective packing".

For example, an attacker could mine a block with a timestamp that is slightly in the future, allowing them to bypass a time-based restriction in a smart contract that relies on block.timestamp. This could potentially allow the attacker to execute a malicious action that would otherwise be blocked by the restriction.

It is reccomended to, instead, use an alternative timestamp source, such as an oracle, that is not susceptible to manipulation by a miner.

Findings

• src/Lottery.sol

  ::135 =>         if (block.timestamp < drawScheduledAt(currentDraw)) {
  ::164 =>             if (block.timestamp <= ticketRegistrationDeadline(ticketInfo.drawId + LotteryMath.DRAWS_PER_YEAR)) {

• src/LotterySetup.sol

  ::74 =>         if (initialPotDeadline < (block.timestamp + lotterySetupParams.drawSchedule.drawPeriod)) {
  ::114 =>         if (block.timestamp > ticketRegistrationDeadline(drawId)) {
  ::137 =>         if (block.timestamp <= initialPotDeadline) {

• src/RNSourceController.sol

  ::64 =>         if (block.timestamp - lastRequestTimestamp <= maxRequestDelay) {
  ::70 =>             maxFailedAttemptsReachedAt = block.timestamp;
  ::94 =>         bool notEnoughTimeReachingMaxFailedAttempts = block.timestamp < maxFailedAttemptsReachedAt + maxRequestDelay;
  ::107 =>         lastRequestTimestamp = block.timestamp;

• src/staking/StakedTokenLock.sol

  ::26 =>         if (block.timestamp > depositDeadline) {
  ::39 =>         if (block.timestamp > depositDeadline && block.timestamp < depositDeadline + lockDuration) {

References

• Timestamp dependence | Solidity Best Practices for Smart Contract Security
• What Is Timestamp Dependence?

Non-Critical issues

[NC-01] Unnamed return parameters

Description

To increase explicitness and readability, take into account introducing and utilizing named return parameters.

Findings

• src/LotterySetup.sol#L160

  function _baseJackpot(uint256 _initialPot) internal view returns (uint256) {

References

• Unnamed return parameters | Opyn Bull Strategy Contracts Audit

[NC-02] Pragma Version 0.8.17 too recent to be trusted

Description

In recert versions, unexpected problems might be reported. Use a more robust, non-legacy version like 0.8.10.

Findings

• src/staking/StakedTokenLock.sol => ^0.8.17

References

• Ethereum Solidity changelog
• [N-09] PRAGMA VERSION^0.8.17 VERSION TOO RECENT TO BE TRUSTED.

[NC-03] For modern and more readable code; update import usages

Description

A less obvious way that solidity code is clearer is the struct Point. Prior to now, we imported it via global import, but we didn't use it. The Point struct contaminated the source code with an extra object that was not needed and that we were not utilizing.

To be sure to only import what you need, use specific imports using curly brackets.

Findings

• src/Lottery.sol

  ::5 => import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
  ::6 => import "@openzeppelin/contracts/utils/math/Math.sol";
  ::7 => import "src/ReferralSystem.sol";
  ::8 => import "src/RNSourceController.sol";
  ::9 => import "src/staking/Staking.sol";
  ::10 => import "src/LotterySetup.sol";
  ::11 => import "src/TicketUtils.sol";

• src/LotteryMath.sol

  ::5 => import "src/interfaces/ILottery.sol";
  ::6 => import "src/PercentageMath.sol";

• src/LotterySetup.sol

  ::5 => import "@openzeppelin/contracts/utils/math/Math.sol";
  ::6 => import "@openzeppelin/contracts/token/ERC20/extensions/IERC20Metadata.sol";
  ::7 => import "src/PercentageMath.sol";
  ::8 => import "src/LotteryToken.sol";
  ::9 => import "src/interfaces/ILotterySetup.sol";
  ::10 => import "src/Ticket.sol";

• src/LotteryToken.sol

  ::5 => import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
  ::6 => import "src/interfaces/ILotteryToken.sol";
  ::7 => import "src/LotteryMath.sol";

• src/RNSourceBase.sol

  ::5 => import "src/interfaces/IRNSource.sol";

• src/RNSourceController.sol

  ::5 => import "@openzeppelin/contracts/access/Ownable2Step.sol";
  ::6 => import "src/interfaces/IRNSource.sol";
  ::7 => import "src/interfaces/IRNSourceController.sol";

• src/ReferralSystem.sol

  ::5 => import "@openzeppelin/contracts/utils/math/Math.sol";
  ::6 => import "src/interfaces/IReferralSystem.sol";
  ::7 => import "src/PercentageMath.sol";

• src/Ticket.sol

  ::5 => import "@openzeppelin/contracts/token/ERC721/ERC721.sol";
  ::6 => import "src/interfaces/ITicket.sol";

• src/VRFv2RNSource.sol

  ::5 => import "@chainlink/contracts/src/v0.8/VRFV2WrapperConsumerBase.sol";
  ::6 => import "src/interfaces/IVRFv2RNSource.sol";
  ::7 => import "src/RNSourceBase.sol";

• src/interfaces/ILottery.sol

  ::5 => import "src/interfaces/ILotterySetup.sol";
  ::6 => import "src/interfaces/IRNSourceController.sol";
  ::7 => import "src/interfaces/ITicket.sol";
  ::8 => import "src/interfaces/IReferralSystem.sol";

• src/interfaces/ILotterySetup.sol

  ::5 => import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
  ::6 => import "src/interfaces/ITicket.sol";

• src/interfaces/ILotteryToken.sol

  ::5 => import "@openzeppelin/contracts/token/ERC20/IERC20.sol";

• interfaces/IRNSourceController.sol

  ::5 => import "@openzeppelin/contracts/access/Ownable2Step.sol";
  ::6 => import "src/interfaces/IRNSource.sol";

• src/interfaces/IReferralSystem.sol

  ::5 => import "src/interfaces/ILotteryToken.sol";

• src/interfaces/ITicket.sol

  ::5 => import "@openzeppelin/contracts/token/ERC721/IERC721.sol";

• src/interfaces/IVRFv2RNSource.sol

  ::5 => import "src/interfaces/IRNSource.sol";

• src/staking/StakedTokenLock.sol

  ::5 => import "@openzeppelin/contracts/access/Ownable2Step.sol";
  ::6 => import "src/staking/interfaces/IStakedTokenLock.sol";
  ::7 => import "src/staking/interfaces/IStaking.sol";

• src/staking/Staking.sol

  ::5 => import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
  ::6 => import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
  ::7 => import "src/interfaces/ILottery.sol";
  ::8 => import "src/LotteryMath.sol";
  ::9 => import "src/staking/interfaces/IStaking.sol";

• src/staking/interfaces/IStakedTokenLock.sol

  ::5 => import "src/staking/interfaces/IStaking.sol";

• staking/interfaces/IStaking.sol

  ::5 => import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
  ::6 => import "src/interfaces/ILottery.sol";

References

• [N-03] FOR MODERN AND MORE READABLE CODE; UPDATE IMPORT USAGES | PoolTogether contest