Wenwin contest - Rolezn's results

The next generation of chance-based gaming.

General Information

Platform: Code4rena

Start Date: 06/03/2023

Pot Size: $36,500 USDC

Total HM: 8

Participants: 93

Period: 3 days

Judge: cccz

Total Solo HM: 3

Id: 218

League: ETH

Wenwin

Findings Distribution

Researcher Performance

Rank: 23/93

Findings: 2

Award: $275.63

QA:
grade-a
Gas:
grade-a

🌟 Selected for report: 1

🚀 Solo Findings: 0

Awards

169.7989 USDC - $169.80

Labels

bug
grade-a
QA (Quality Assurance)
sponsor disputed
edited-by-warden
Q-48

External Links

Summary<a name="Summary">

Low Risk Issues

IssueContexts
LOW‑1Add to blacklist function4
LOW‑2decimals() not part of ERC20 standard3
LOW‑3Event is missing parameters1
LOW‑4Possible rounding issue2
LOW‑5Use _safeMint instead of _mint1
LOW‑6Contracts are not using their OZ Upgradeable counterparts16
LOW‑7Remove unused code1
LOW‑8require() should be used instead of assert()2

Total: 30 contexts over 8 issues

Non-critical Issues

IssueContexts
NC‑1Avoid Floating Pragmas: The Version Should Be Locked3
NC‑2Constants Should Be Defined Rather Than Using Magic Numbers3
NC‑3Constants in comparisons should appear on the left side1
NC‑4Function writing that does not comply with the Solidity Style Guide24
NC‑5Use delete to Clear Variables8
NC‑6NatSpec return parameters should be included in contracts1
NC‑7Implementation contract may not be initialized10
NC‑8NatSpec comments should be increased in contracts1
NC‑9Use a more recent version of Solidity3
NC‑10Public Functions Not Called By The Contract Should Be Declared External Instead1
NC‑11Empty blocks should be removed or emit something1
NC‑12Project should implement pausable functionality1

Total: 57 contexts over 12 issues

Low Risk Issues

<a href="#Summary">[LOW‑1]</a><a name="LOW&#x2011;1"> Add to blacklist function

NFT thefts have increased recently, so with the addition of hacked NFT lottery tickets to the platform, NFTs can be converted into liquidity. To prevent this, I recommend adding the blacklist function.

Marketplaces such as Opensea have a blacklist feature that will not list NFTs that have been reported theft, NFT projects such as Manifold have blacklist functions in their smart contracts.

Here is the project example; Manifold

Manifold Contract https://etherscan.io/address/0xe4e4003afe3765aca8149a82fc064c0b125b9e5a#code

     modifier nonBlacklistRequired(address extension) {
         require(!_blacklistedExtensions.contains(extension), "Extension blacklisted");
         _;
     }
<ins>Recommended Mitigation Steps</ins>

Add to Blacklist function and modifier.

<a href="#Summary">[LOW‑2]</a><a name="LOW&#x2011;2"> decimals() not part of ERC20 standard

decimals() is not part of the official ERC20 standard and might fail for tokens that do not implement it. While in practice it is very unlikely, as usually most of the tokens implement it, this should still be considered as a potential issue.

<ins>Proof Of Concept</ins>
79: uint256 tokenUnit = 10 ** IERC20Metadata(address(lotterySetupParams.token)).decimals()

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/LotterySetup.sol#L79

128: return extracted * (10 ** (IERC20Metadata(address(rewardToken)).decimals()

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/LotterySetup.sol#L128

168: uint256 divisor = 10 ** (IERC20Metadata(address(rewardToken)).decimals()

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/LotterySetup.sol#L168

<a href="#Summary">[LOW‑3]</a><a name="LOW&#x2011;3"> Event is missing parameters

The following functions are missing critical parameters when emitting an event. When dealing with source address which uses the value of msg.sender, the msg.sender value must be specified in every transaction, a contract or web page listening to events cannot react to users, emit does not serve the purpose. Basically, this event cannot be used.

<ins>Proof Of Concept</ins>

function claimRewards(LotteryRewardType rewardType) external override returns (uint256 claimedAmount) {
        address beneficiary = (rewardType == LotteryRewardType.FRONTEND) ? msg.sender : stakingRewardRecipient;
        claimedAmount = LotteryMath.calculateRewards(ticketPrice, dueTicketsSoldAndReset(beneficiary), rewardType);

        emit ClaimedRewards(beneficiary, claimedAmount, rewardType);
        rewardToken.safeTransfer(beneficiary, claimedAmount);
    }

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/Lottery.sol#L155

<ins>Recommended Mitigation Steps</ins>

Add msg.sender parameter in event-emit

<a href="#Summary">[LOW‑4]</a><a name="LOW&#x2011;4"> Possible rounding issue

Division by large numbers may result in the result being zero, due to solidity not supporting fractions. Consider requiring a minimum amount for the numerator to ensure that it is always larger than the denominator

<ins>Proof Of Concept</ins>
100: referrerRewardForDraw / totalTicketsForReferrersPerCurrentDraw;
105: playerRewardsPerDrawForOneTicket[drawFinalized] = playerRewardForDraw / ticketsSoldDuringDraw;

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/ReferralSystem.sol#L100

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/ReferralSystem.sol#L105

<a href="#Summary">[LOW‑5]</a><a name="LOW&#x2011;5"> Use _safeMint instead of _mint

According to openzepplin's ERC721, the use of _mint is discouraged, use _safeMint whenever possible. https://docs.openzeppelin.com/contracts/3.x/api/token/erc721#ERC721-_mint-address-uint256-

<ins>Proof Of Concept</ins>
26: _mint(to, ticketId);

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/Ticket.sol#L26

<ins>Recommended Mitigation Steps</ins>

Use _safeMint whenever possible instead of _mint

<a href="#Summary">[LOW‑6]</a><a name="LOW&#x2011;6"> Contracts are not using their OZ Upgradeable counterparts

The non-upgradeable standard version of OpenZeppelin’s library are inherited / used by the contracts. It would be safer to use the upgradeable versions of the library contracts to avoid unexpected behaviour.

<ins>Proof of Concept</ins>
5: import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
6: import "@openzeppelin/contracts/utils/math/Math.sol";

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/Lottery.sol#L5-L6

5: import "@openzeppelin/contracts/utils/math/Math.sol";
6: import "@openzeppelin/contracts/token/ERC20/extensions/IERC20Metadata.sol";

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/LotterySetup.sol#L5-L6

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

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/LotteryToken.sol#L5

5: import "@openzeppelin/contracts/utils/math/Math.sol";

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/ReferralSystem.sol#L5

5: import "@openzeppelin/contracts/access/Ownable2Step.sol";

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/RNSourceController.sol#L5

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

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/Ticket.sol#L5

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

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/interfaces/ILotterySetup.sol#L5

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

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/interfaces/ILotteryToken.sol#L5

5: import "@openzeppelin/contracts/access/Ownable2Step.sol";

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/interfaces/IRNSourceController.sol#L5

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

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/interfaces/ITicket.sol#L5

5: import "@openzeppelin/contracts/access/Ownable2Step.sol";

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/staking/StakedTokenLock.sol#L5

5: import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
6: import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/staking/Staking.sol#L5-L6

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

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/staking/interfaces/IStaking.sol#L5

<ins>Recommended Mitigation Steps</ins>

Where applicable, use the contracts from @openzeppelin/contracts-upgradeable instead of @openzeppelin/contracts. See https://github.com/OpenZeppelin/openzeppelin-contracts-upgradeable/tree/master/contracts for list of available upgradeable contracts

<a href="#Summary">[LOW‑7]</a><a name="LOW&#x2011;7"> Remove unused code

This code is not used in the main project contract files, remove it or add event-emit Code that is not in use, suggests that they should not be present and could potentially contain insecure functionalities.

<ins>Proof Of Concept</ins>
function fulfillRandomWords

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/VRFv2RNSource.sol#L32

<a href="#Summary">[LOW‑8]</a><a name="LOW&#x2011;8"> require() Should Be Used Instead Of assert()

Prior to solidity version 0.8.0, hitting an assert consumes the remainder of the transaction's available gas rather than returning it, as require()/revert() do. assert() should be avoided even past solidity version 0.8.0 as its <a href="https://docs.soliditylang.org/en/v0.8.14/control-structures.html#panic-via-assert-and-error-via-require">documentation</a> states that "The assert function creates an error of type Panic(uint256). ... Properly functioning code should never create a Panic, not even on invalid external input. If this happens, then there is a bug in your contract which you should fix".

<ins>Proof Of Concept</ins>
147: assert(initialPot > 0);

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/LotterySetup.sol#L147

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

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/TicketUtils.sol#L99

<a href="#Summary">[LOW‑12]</a><a name="LOW&#x2011;12"> Admin privilege - A single point of failure can allow a hacked or malicious owner use critical functions in the project

The owner role has a single point of failure and onlyOwner can use critical a few functions.

owner role in the project: Owner is not behind a multisig and changes are not behind a timelock.

Even if protocol admins/developers are not malicious there is still a chance for Owner keys to be stolen. In such a case, the attacker can cause serious damage to the project due to important functions. In such a case, users who have invested in project will suffer high financial losses.

Impact

Hacked owner or malicious owner can immediately use critical functions in the project.

<ins>Proof Of Concept</ins>
77: function initSource(IRNSource rnSource) external override onlyOwner {

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/RNSourceController.sol#L77

89: function swapSource(IRNSource newSource) external override onlyOwner {

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/RNSourceController.sol#L89

24: function deposit(uint256 amount) external override onlyOwner {

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/staking/StakedTokenLock.sol#L24

37: function withdraw(uint256 amount) external override onlyOwner {

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/staking/StakedTokenLock.sol#L37

<ins>Recommended Mitigation Steps</ins>

Add a time lock to critical functions. Admin-only functions that change critical parameters should emit events and have timelocks. Events allow capturing the changed parameters so that off-chain tools/interfaces can register such changes with timelocks that allow users to evaluate them and consider if they would like to engage/exit based on how they perceive the changes as affecting the trustworthiness of the protocol or profitability of the implemented financial services.

Allow only multi-signature wallets to call the function to reduce the likelihood of an attack.

https://twitter.com/danielvf/status/1572963475101556738?s=20&t=V1kvzfJlsx-D2hfnG0OmuQ

Reference

The status quo regarding significant centralization vectors has always been to award M severity but I have lowered it to QA as it is a common finding this is also in order to at least warn users of the protocol of this category of risks. See <a href="https://gist.github.com/GalloDaSballo/881e7a45ac14481519fb88f34fdb8837">here</a> for list of centralization issues previously judged.

Non Critical Issues

<a href="#Summary">[NC‑1]</a><a name="NC&#x2011;1"> Avoid Floating Pragmas: The Version Should Be Locked

Avoid floating pragmas for non-library contracts.

While floating pragmas make sense for libraries to allow them to be included with multiple different versions of applications, it may be a security risk for application implementations.

A known vulnerable compiler version may accidentally be selected or security tools might fall-back to an older compiler version ending up checking a different EVM compilation that is ultimately deployed on the blockchain.

It is recommended to pin to a concrete compiler version.

<ins>Proof Of Concept</ins>
Found usage of floating pragmas ^0.8.7 of Solidity in [VRFv2RNSource.sol]

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/VRFv2RNSource.sol#L3

Found usage of floating pragmas ^0.8.7 of Solidity in [IVRFv2RNSource.sol]

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/interfaces/IVRFv2RNSource.sol#L3

Found usage of floating pragmas ^0.8.17 of Solidity in [StakedTokenLock.sol]

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/staking/StakedTokenLock.sol#L3

<a href="#Summary">[NC‑2]</a><a name="NC&#x2011;2"> Constants Should Be Defined Rather Than Using Magic Numbers

<ins>Proof Of Concept</ins>
117: if (totalTicketsSoldPrevDraw < 10_000) {

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/ReferralSystem.sol#L117

121: if (totalTicketsSoldPrevDraw < 100_000) {

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/ReferralSystem.sol#L121

125: if (totalTicketsSoldPrevDraw < 1_000_000) {

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/ReferralSystem.sol#L125

<a href="#Summary">[NC‑3]</a><a name="NC&#x2011;3"> Constants in comparisons should appear on the left side

Doing so will prevent <a href="https://www.moserware.com/2008/01/constants-on-left-are-better-but-this.html">typo bugs</a>

<ins>Proof Of Concept</ins>
50: if (_totalSupply == 0) {

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/staking/Staking.sol#L50

<a href="#Summary">[NC‑4]</a><a name="NC&#x2011;4"> Function writing that does not comply with the Solidity Style Guide

Order of Functions; ordering helps readers identify which functions they can call and to find the constructor and fallback definitions easier. But there are contracts in the project that do not comply with this.

https://docs.soliditylang.org/en/v0.8.17/style-guide.html

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
<ins>Proof Of Concept</ins>

All in-scope contracts

For example: See Lottery.sol

<a href="#Summary">[NC‑5]</a><a name="NC&#x2011;5"> Use delete to Clear Variables

delete a assigns the initial value for the type to a. i.e. for integers it is equivalent to a = 0, but it can also be used on arrays, where it assigns a dynamic array of length zero or a static array of the same length with all elements reset. For structs, it assigns a struct with all members reset. Similarly, it can also be used to set an address to zero address. It has no effect on whole mappings though (as the keys of mappings may be arbitrary and are generally unknown). However, individual keys and what they map to can be deleted: If a is a mapping, then delete a[x] will delete the value stored at x.

The delete key better conveys the intention and is also more idiomatic. Consider replacing assignments of zero with delete statements.

<ins>Proof Of Concept</ins>
255: frontendDueTicketSales[beneficiary] = 0;

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/Lottery.sol#L255

142: unclaimedTickets[drawId][msg.sender].referrerTicketCount = 0;
148: unclaimedTickets[drawId][msg.sender].playerTicketCount = 0;

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/ReferralSystem.sol#L142

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/ReferralSystem.sol#L148

52: failedSequentialAttempts = 0;
53: maxFailedAttemptsReachedAt = 0;

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/RNSourceController.sol#L52-L53

99: failedSequentialAttempts = 0;
100: maxFailedAttemptsReachedAt = 0;

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/RNSourceController.sol#L99-L100

95: rewards[msg.sender] = 0;

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/staking/Staking.sol#L95

<a href="#Summary">[NC‑6]</a><a name="NC&#x2011;6"> NatSpec return parameters should be included in contracts

It is recommended that Solidity contracts are fully annotated using NatSpec for all public interfaces (everything in the ABI). It is clearly stated in the Solidity official documentation. In complex projects such as Defi, the interpretation of all functions and their arguments and returns is important for code readability and auditability.

https://docs.soliditylang.org/en/v0.8.15/natspec-format.html

<ins>Proof Of Concept</ins>

All in-scope contracts

For example: See Lottery.sol for function registerTicket(

<ins>Recommended Mitigation Steps</ins>

Include return parameters in NatSpec comments

Recommendation Code Style: (from Uniswap3)

    /// @notice Adds liquidity for the given recipient/tickLower/tickUpper position
    /// @dev The caller of this method receives a callback in the form of IUniswapV3MintCallback#uniswapV3MintCallback
    /// in which they must pay any token0 or token1 owed for the liquidity. The amount of token0/token1 due depends
    /// on tickLower, tickUpper, the amount of liquidity, and the current price.
    /// @param recipient The address for which the liquidity will be created
    /// @param tickLower The lower tick of the position in which to add liquidity
    /// @param tickUpper The upper tick of the position in which to add liquidity
    /// @param amount The amount of liquidity to mint
    /// @param data Any data that should be passed through to the callback
    /// @return amount0 The amount of token0 that was paid to mint the given amount of liquidity. Matches the value in the callback
    /// @return amount1 The amount of token1 that was paid to mint the given amount of liquidity. Matches the value in the callback
    function mint(
        address recipient,
        int24 tickLower,
        int24 tickUpper,
        uint128 amount,
        bytes calldata data
    ) external returns (uint256 amount0, uint256 amount1);

<a href="#Summary">[NC‑7]</a><a name="NC&#x2011;7"> Implementation contract may not be initialized

OpenZeppelin recommends that the initializer modifier be applied to constructors. Per OZs Post implementation contract should be initialized to avoid potential griefs or exploits. https://forum.openzeppelin.com/t/uupsupgradeable-vulnerability-post-mortem/15680/5

<ins>Proof Of Concept</ins>
84: constructor(
        LotterySetupParams memory lotterySetupParams,
        uint256 playerRewardFirstDraw,
        uint256 playerRewardDecreasePerDraw,
        uint256[] memory rewardsToReferrersPerDraw,
        uint256 maxRNFailedAttempts,
        uint256 maxRNRequestDelay
    )
        Ticket()
        LotterySetup(lotterySetupParams)
        ReferralSystem(playerRewardFirstDraw, playerRewardDecreasePerDraw, rewardsToReferrersPerDraw)
        RNSourceController(maxRNFailedAttempts, maxRNRequestDelay)

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/Lottery.sol#L84

41: constructor(LotterySetupParams memory lotterySetupParams)

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/LotterySetup.sol#L41

17: constructor() ERC20("Wenwin Lottery", "LOT")

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/LotteryToken.sol#L17

27: constructor(
        uint256 _playerRewardFirstDraw,
        uint256 _playerRewardDecreasePerDraw,
        uint256[] memory _rewardsToReferrersPerDraw
    )

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/ReferralSystem.sol#L27

11: constructor(address _authorizedConsumer)

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/RNSourceBase.sol#L11

26: constructor(uint256 _maxFailedAttempts, uint256 _maxRequestDelay)

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/RNSourceController.sol#L26

17: constructor() ERC721("Wenwin Lottery Ticket", "WLT")

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/Ticket.sol#L17

13: constructor(
        address _authorizedConsumer,
        address _linkAddress,
        address _wrapperAddress,
        uint16 _requestConfirmations,
        uint32 _callbackGasLimit
    )
        RNSourceBase(_authorizedConsumer)
        VRFV2WrapperConsumerBase(_linkAddress, _wrapperAddress)

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/VRFv2RNSource.sol#L13

16: constructor(address _stakedToken, uint256 _depositDeadline, uint256 _lockDuration)

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/staking/StakedTokenLock.sol#L16

22: constructor(
        ILottery _lottery,
        IERC20 _rewardsToken,
        IERC20 _stakingToken,
        string memory name,
        string memory symbol
    )
        ERC20(name, symbol)

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/staking/Staking.sol#L22

<a href="#Summary">[NC‑8]</a><a name="NC&#x2011;8"> NatSpec comments should be increased in contracts

It is recommended that Solidity contracts are fully annotated using NatSpec for all public interfaces (everything in the ABI). It is clearly stated in the Solidity official documentation. In complex projects such as Defi, the interpretation of all functions and their arguments and returns is important for code readability and auditability. https://docs.soliditylang.org/en/v0.8.15/natspec-format.html

<ins>Proof Of Concept</ins>

All in-scope contracts

For example: See Lottery.sol

<ins>Recommended Mitigation Steps</ins>

NatSpec comments should be increased in contracts

<a href="#Summary">[NC‑9]</a><a name="NC&#x2011;9"> Use a more recent version of Solidity

<a href="https://blog.soliditylang.org/2022/02/16/solidity-0.8.12-release-announcement/">0.8.12</a>: string.concat() instead of abi.encodePacked(<str>,<str>)

<a href="https://blog.soliditylang.org/2022/03/16/solidity-0.8.13-release-announcement/">0.8.13</a>: Ability to use using for with a list of free functions

<a href="https://blog.soliditylang.org/2022/05/18/solidity-0.8.14-release-announcement/">0.8.14</a>:

ABI Encoder: When ABI-encoding values from calldata that contain nested arrays, correctly validate the nested array length against calldatasize() in all cases. Override Checker: Allow changing data location for parameters only when overriding external functions.

<a href="https://blog.soliditylang.org/2022/06/15/solidity-0.8.15-release-announcement/">0.8.15</a>:

Code Generation: Avoid writing dirty bytes to storage when copying bytes arrays. Yul Optimizer: Keep all memory side-effects of inline assembly blocks.

<a href="https://blog.soliditylang.org/2022/08/08/solidity-0.8.16-release-announcement/">0.8.16</a>:

Code Generation: Fix data corruption that affected ABI-encoding of calldata values represented by tuples: structs at any nesting level; argument lists of external functions, events and errors; return value lists of external functions. The 32 leading bytes of the first dynamically-encoded value in the tuple would get zeroed when the last component contained a statically-encoded array.

<a href="https://blog.soliditylang.org/2022/09/08/solidity-0.8.17-release-announcement/">0.8.17</a>:

Yul Optimizer: Prevent the incorrect removal of storage writes before calls to Yul functions that conditionally terminate the external EVM call.

<a href="https://blog.soliditylang.org/2023/02/22/solidity-0.8.19-release-announcement/">0.8.19</a>: SMTChecker: New trusted mode that assumes that any compile-time available code is the actual used code, even in external calls. Bug Fixes:

  • Assembler: Avoid duplicating subassembly bytecode where possible.
  • Code Generator: Avoid including references to the deployed label of referenced functions if they are called right away.
  • ContractLevelChecker: Properly distinguish the case of missing base constructor arguments from having an unimplemented base function.
  • SMTChecker: Fix internal error caused by unhandled z3 expressions that come from the solver when bitwise operators are used.
  • SMTChecker: Fix internal error when using the custom NatSpec annotation to abstract free functions.
  • TypeChecker: Also allow external library functions in using for.
<ins>Proof Of Concept</ins>
pragma solidity ^0.8.7;

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/VRFv2RNSource.sol#L3

pragma solidity ^0.8.7;

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/interfaces/IVRFv2RNSource.sol#L3

pragma solidity ^0.8.17;

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/staking/StakedTokenLock.sol#L3

<ins>Recommended Mitigation Steps</ins>

Consider updating to a more recent solidity version.

<a href="#Summary">[NC‑10]</a><a name="NC&#x2011;10"> Public Functions Not Called By The Contract Should Be Declared External Instead

Contracts are allowed to override their parents’ functions and change the visibility from external to public.

<ins>Proof Of Concept</ins>
function currentRewardSize(uint8 winTier) public view override returns (uint256 rewardSize) {

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/Lottery.sol#L234

<a href="#Summary">[NC‑11]</a><a name="NC&#x2011;11"> Empty blocks should be removed or emit something

<ins>Proof Of Concept</ins>
17: constructor() ERC721("Wenwin Lottery Ticket", "WLT") { }

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/Ticket.sol#L17

<a href="#Summary">[NC‑12]</a><a name="NC&#x2011;12"> Project should implement pausable functionality

The project should implement a pausing function to allow pausing should an critical issues be discovered, hence pausing any changes that would happen until a fix has been implemented.

See OZ's Pausable contract for more information: https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/security/Pausable.sol

#0 - thereksfour

2023-03-12T12:25:51Z

3 L 4 INFO

#1 - c4-judge

2023-03-12T12:25:55Z

thereksfour marked the issue as grade-a

#2 - c4-sponsor

2023-03-14T11:39:56Z

0xluckydev marked the issue as sponsor disputed

#3 - thereksfour

2023-03-17T13:19:33Z

3L 4 INFO A

Awards

105.8343 USDC - $105.83

Labels

bug
G (Gas Optimization)
grade-a
selected for report
sponsor confirmed
edited-by-warden
G-21

External Links

Summary<a name="Summary">

Gas Optimizations

IssueContextsEstimated Gas Saved
GAS‑1Use calldata instead of memory for function parameters2600
GAS‑2Setting the constructor to payable10130
GAS‑3Do not calculate constants6-
GAS‑4Using delete statement can save gas8-
GAS‑5Functions guaranteed to revert when called by normal users can be marked payable484
GAS‑6Use hardcoded address instead address(this)5-
GAS‑7internal functions only called once can be inlined to save gas488
GAS‑8Multiple Address Mappings Can Be Combined Into A Single Mapping Of An Address To A Struct, Where Appropriate2-
GAS‑9Optimize names to save gas10220
GAS‑10<x> += <y> Costs More Gas Than <x> = <x> + <y> For State Variables16-
GAS‑11Public Functions To External8-
GAS‑12require() Should Be Used Instead Of assert()2-
GAS‑13Shorten the array rather than copying to a new one2-
GAS‑14Help The Optimizer By Saving A Storage Variable’s Reference Instead Of Repeatedly Fetching It3-
GAS‑15Structs can be packed into fewer storage slots48000
GAS‑16Usage of uints/ints smaller than 32 bytes (256 bits) incurs overhead21-
GAS‑17Unnecessary look up in if condition12100
GAS‑18Use solidity version 0.8.19 to gain some gas boost3264

Total: 109 contexts over 18 issues

Gas Optimizations

<a href="#Summary">[GAS‑1]</a><a name="GAS&#x2011;1"> Use calldata instead of memory for function parameters

In some cases, having function arguments in calldata instead of memory is more optimal.

Consider the following generic example:

contract C { function add(uint[] memory arr) external returns (uint sum) { uint length = arr.length; for (uint i = 0; i < arr.length; i++) { sum += arr[i]; } } }

In the above example, the dynamic array arr has the storage location memory. When the function gets called externally, the array values are kept in calldata and copied to memory during ABI decoding (using the opcode calldataload and mstore). And during the for loop, arr[i] accesses the value in memory using a mload. However, for the above example this is inefficient. Consider the following snippet instead:

contract C { function add(uint[] calldata arr) external returns (uint sum) { uint length = arr.length; for (uint i = 0; i < arr.length; i++) { sum += arr[i]; } } }

In the above snippet, instead of going via memory, the value is directly read from calldata using calldataload. That is, there are no intermediate memory operations that carries this value.

Gas savings: In the former example, the ABI decoding begins with copying value from calldata to memory in a for loop. Each iteration would cost at least 60 gas. In the latter example, this can be completely avoided. This will also reduce the number of instructions and therefore reduces the deploy time cost of the contract.

In short, use calldata instead of memory if the function argument is only read.

Note that in older Solidity versions, changing some function arguments from memory to calldata may cause "unimplemented feature error". This can be avoided by using a newer (0.8.*) Solidity compiler.

Examples Note: The following pattern is prevalent in the codebase:

function f(bytes memory data) external { (...) = abi.decode(data, (..., types, ...)); }

Here, changing to bytes calldata will decrease the gas. The total savings for this change across all such uses would be quite significant.

<ins>Proof Of Concept</ins>
function packFixedRewards(uint256[] memory rewards) private view returns (uint256 packed) {

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/LotterySetup.sol#L164

function fulfillRandomWords(uint256 requestId, uint256[] memory randomWords) internal override {

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/VRFv2RNSource.sol#L32

<a href="#Summary">[GAS‑2]</a><a name="GAS&#x2011;2"> Setting the constructor to payable

Saves ~13 gas per instance

<ins>Proof Of Concept</ins>
84: constructor(
        LotterySetupParams memory lotterySetupParams,
        uint256 playerRewardFirstDraw,
        uint256 playerRewardDecreasePerDraw,
        uint256[] memory rewardsToReferrersPerDraw,
        uint256 maxRNFailedAttempts,
        uint256 maxRNRequestDelay
    )
        Ticket()
        LotterySetup(lotterySetupParams)
        ReferralSystem(playerRewardFirstDraw, playerRewardDecreasePerDraw, rewardsToReferrersPerDraw)
        RNSourceController(maxRNFailedAttempts, maxRNRequestDelay)

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/Lottery.sol#L84

41: constructor(LotterySetupParams memory lotterySetupParams)

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/LotterySetup.sol#L41

17: constructor() ERC20("Wenwin Lottery", "LOT")

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/LotteryToken.sol#L17

27: constructor(
        uint256 _playerRewardFirstDraw,
        uint256 _playerRewardDecreasePerDraw,
        uint256[] memory _rewardsToReferrersPerDraw
    )

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/ReferralSystem.sol#L27

11: constructor(address _authorizedConsumer)

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/RNSourceBase.sol#L11

26: constructor(uint256 _maxFailedAttempts, uint256 _maxRequestDelay)

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/RNSourceController.sol#L26

17: constructor() ERC721("Wenwin Lottery Ticket", "WLT")

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/Ticket.sol#L17

13: constructor(
        address _authorizedConsumer,
        address _linkAddress,
        address _wrapperAddress,
        uint16 _requestConfirmations,
        uint32 _callbackGasLimit
    )
        RNSourceBase(_authorizedConsumer)
        VRFV2WrapperConsumerBase(_linkAddress, _wrapperAddress)

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/VRFv2RNSource.sol#L13

16: constructor(address _stakedToken, uint256 _depositDeadline, uint256 _lockDuration)

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/staking/StakedTokenLock.sol#L16

22: constructor(
        ILottery _lottery,
        IERC20 _rewardsToken,
        IERC20 _stakingToken,
        string memory name,
        string memory symbol
    )
        ERC20(name, symbol)

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/staking/Staking.sol#L22

<a href="#Summary">[GAS‑3]</a><a name="GAS&#x2011;3"> Do not calculate constants

Due to how constant variables are implemented (replacements at compile-time), an expression assigned to a constant variable is recomputed each time that the variable is used, which wastes some gas.

<ins>Proof Of Concept</ins>
14: uint256 public constant STAKING_REWARD = 20 * PercentageMath.ONE_PERCENT;

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/LotteryMath.sol#L14

16: uint256 public constant FRONTEND_REWARD = 10 * PercentageMath.ONE_PERCENT;

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/LotteryMath.sol#L16

18: uint256 public constant TICKET_PRICE_TO_POT = PercentageMath.PERCENTAGE_BASE - STAKING_REWARD - FRONTEND_REWARD;

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/LotteryMath.sol#L18

20: uint256 public constant SAFETY_MARGIN = 67 * PercentageMath.ONE_PERCENT;

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/LotteryMath.sol#L20

22: uint256 public constant EXCESS_BONUS_ALLOCATION = 50 * PercentageMath.ONE_PERCENT;

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/LotteryMath.sol#L22

36: uint256 private constant BASE_JACKPOT_PERCENTAGE = 30_030; // 30.03%

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/LotterySetup.sol#L36

<a href="#Summary">[GAS‑4]</a><a name="GAS&#x2011;4"> Using delete statement can save gas

<ins>Proof Of Concept</ins>
255: frontendDueTicketSales[beneficiary] = 0;

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/Lottery.sol#L255

142: unclaimedTickets[drawId][msg.sender].referrerTicketCount = 0;
148: unclaimedTickets[drawId][msg.sender].playerTicketCount = 0;

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/ReferralSystem.sol#L142

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/ReferralSystem.sol#L148

52: failedSequentialAttempts = 0;
53: maxFailedAttemptsReachedAt = 0;

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/RNSourceController.sol#L52-L53

99: failedSequentialAttempts = 0;
100: maxFailedAttemptsReachedAt = 0;

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/RNSourceController.sol#L99-L100

95: rewards[msg.sender] = 0;

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/staking/Staking.sol#L95

<a href="#Summary">[GAS‑5]</a><a name="GAS&#x2011;5"> Functions guaranteed to revert when called by normal users can be marked payable

If a function modifier or require such as onlyOwner/onlyX is used, the function will revert if a normal user tries to pay the function. Marking the function as payable will lower the gas cost for legitimate callers because the compiler will not include checks for whether a payment was provided. The extra opcodes avoided are CALLVALUE(2), DUP1(3), ISZERO(3), PUSH2(3), JUMPI(10), PUSH1(3), DUP1(3), REVERT(0), JUMPDEST(1), POP(2) which costs an average of about 21 gas per call to the function, in addition to the extra deployment cost.

<ins>Proof Of Concept</ins>
77: function initSource(IRNSource rnSource) external override onlyOwner {

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/RNSourceController.sol#L77

89: function swapSource(IRNSource newSource) external override onlyOwner {

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/RNSourceController.sol#L89

24: function deposit(uint256 amount) external override onlyOwner {

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/staking/StakedTokenLock.sol#L24

37: function withdraw(uint256 amount) external override onlyOwner {

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/staking/StakedTokenLock.sol#L37

<ins>Recommended Mitigation Steps</ins>

Functions guaranteed to revert when called by normal users can be marked payable.

<a href="#Summary">[GAS‑6]</a><a name="GAS&#x2011;6"> Use hardcode address instead address(this)

Instead of using address(this), it is more gas-efficient to pre-calculate and use the hardcoded address. Foundry's script.sol and solmate's LibRlp.sol contracts can help achieve this.

References: https://book.getfoundry.sh/reference/forge-std/compute-create-address

https://twitter.com/transmissions11/status/1518507047943245824

<ins>Proof Of Concept</ins>
130: rewardToken.safeTransferFrom(msg.sender, address(this), ticketPrice * tickets.length);

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/Lottery.sol#L130

140: uint256 raised = rewardToken.balanceOf(address(this));

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/LotterySetup.sol#L140

34: stakedToken.transferFrom(msg.sender, address(this), amount);

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/staking/StakedTokenLock.sol#L34

55: rewardsToken.transfer(owner(), rewardsToken.balanceOf(address(this)));

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/staking/StakedTokenLock.sol#L55

73: stakingToken.safeTransferFrom(msg.sender, address(this), amount);

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/staking/Staking.sol#L73

<ins>Recommended Mitigation Steps</ins>

Use hardcoded address

<a href="#Summary">[GAS‑7]</a><a name="GAS&#x2011;7"> internal functions only called once can be inlined to save gas

<ins>Proof Of Concept</ins>
203: function receiveRandomNumber

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/Lottery.sol#L203

279: function requireFinishedDraw

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/Lottery.sol#L279

285: function mintNativeTokens

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/Lottery.sol#L285

160: function _baseJackpot

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/LotterySetup.sol#L160

<a href="#Summary">[GAS‑8]</a><a name="GAS&#x2011;8"> Multiple Address Mappings Can Be Combined Into A Single Mapping Of An Address To A Struct, Where Appropriate

Saves a storage slot for the mapping. Depending on the circumstances and sizes of types, can avoid a Gsset (20000 gas) per mapping combined. Reads and subsequent writes can also be cheaper when a function requires both values and they both fit in the same storage slot.

<ins>Proof Of Concept</ins>
19: mapping(address => uint256) public override userRewardPerTokenPaid;
20: mapping(address => uint256) public override rewards;

//@audit Can be edited to the following struct:
struct addressRewardsStruct {
    uint256 userRewardPerTokenPaid;
    uint256 rewards;
}

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/staking/Staking.sol#L20

<a href="#Summary">[GAS‑9]</a><a name="GAS&#x2011;9"> Optimize names to save gas

Contracts most called functions could simply save gas by function ordering via Method ID. Calling a function at runtime will be cheaper if the function is positioned earlier in the order (has a relatively lower Method ID) because 22 gas are added to the cost of a function for every position that came before it. The caller can save on gas if you prioritize most called functions.

See more <a href="https://medium.com/joyso/solidity-how-does-function-name-affect-gas-consumption-in-smart-contract-47d270d8ac92">here</a>

<ins>Proof Of Concept</ins>

All in-scope contracts

<ins>Recommended Mitigation Steps</ins>

Find a lower method ID name for the most called functions for example Call() vs. Call1() is cheaper by 22 gas For example, the function IDs in the Gauge.sol contract will be the most used; A lower method ID may be given.

<a href="#Summary">[GAS‑10]</a><a name="GAS&#x2011;10"> <x> += <y> Costs More Gas Than <x> = <x> + <y> For State Variables

<ins>Proof Of Concept</ins>
129: frontendDueTicketSales[frontend] += tickets.length;

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/Lottery.sol#L129

173: claimedAmount += claimWinningTicket(ticketIds[i]);

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/Lottery.sol#L173

275: currentNetProfit += int256(unclaimedJackpotTickets * winAmount[drawId][selectionSize]);

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/Lottery.sol#L275

55: newProfit -= int256(expectedRewardsOut);

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/LotteryMath.sol#L55

64: excessPotInt -= int256(fixedJackpotSize);

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/LotteryMath.sol#L64

84: bonusMulti += (excessPot * EXCESS_BONUS_ALLOCATION) / (ticketsSold * expectedPayout);

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/LotteryMath.sol#L84

64: totalTicketsForReferrersPerDraw[currentDraw] +=
67: totalTicketsForReferrersPerDraw[currentDraw] += numberOfTickets;
69: unclaimedTickets[currentDraw][referrer].referrerTicketCount += uint128(numberOfTickets);
71: unclaimedTickets[currentDraw][player].playerTicketCount += uint128(numberOfTickets);

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/ReferralSystem.sol#L64

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/ReferralSystem.sol#L67

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/ReferralSystem.sol#L69

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/ReferralSystem.sol#L71

78: claimedReward += claimPerDraw(drawIds[counter]);

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/ReferralSystem.sol#L78

147: claimedReward += playerRewardsPerDrawForOneTicket[drawId] * _unclaimedTickets.playerTicketCount;

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/ReferralSystem.sol#L147

29: ticketSize += (ticket & uint256(1));

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/TicketUtils.sol#L29

96: winTier += uint8(intersection & uint120(1));

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/TicketUtils.sol#L96

30: depositedBalance += amount;

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/staking/StakedTokenLock.sol#L30

43: depositedBalance -= amount;

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/staking/StakedTokenLock.sol#L43

<a href="#Summary">[GAS‑11]</a><a name="GAS&#x2011;11"> Public Functions To External

The following functions could be set external to save gas and improve code quality. External call cost is less expensive than of public functions.

<ins>Proof Of Concept</ins>
function currentRewardSize(uint8 winTier) public view override returns (uint256 rewardSize) {

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/Lottery.sol#L234

function fixedReward(uint8 winTier) public view override returns (uint256 amount) {

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/LotterySetup.sol#L120

function drawScheduledAt(uint128 drawId) public view override returns (uint256 time) {

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/LotterySetup.sol#L152

function ticketRegistrationDeadline(uint128 drawId) public view override returns (uint256 time) {

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/LotterySetup.sol#L156

function rewardPerToken() public view override returns (uint256 _rewardPerToken) {

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/staking/Staking.sol#L48

function earned(address account) public view override returns (uint256 _earned) {

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/staking/Staking.sol#L61

function withdraw(uint256 amount) public override {

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/staking/Staking.sol#L79

function getReward() public override {

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/staking/Staking.sol#L91

<a href="#Summary">[GAS‑12]</a><a name="GAS&#x2011;12"> require() Should Be Used Instead Of assert()

<ins>Proof Of Concept</ins>
147: assert(initialPot > 0);

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/LotterySetup.sol#L147

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

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/TicketUtils.sol#L99

<a href="#Summary">[GAS‑13]</a><a name="GAS&#x2011;13"> Shorten the array rather than copying to a new one

Inline-assembly can be used to shorten the array by changing the length slot, so that the entries don't have to be copied to a new, shorter array

<ins>Proof Of Concept</ins>
54: uint8[] memory numbers = new uint8[](selectionSize);
63: bool[] memory selected = new bool[](selectionMax);

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/TicketUtils.sol#L54

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/TicketUtils.sol#L63

<a href="#Summary">[GAS‑14]</a><a name="GAS&#x2011;14"> Help The Optimizer By Saving A Storage Variable's Reference Instead Of Repeatedly Fetching It

To help the optimizer, declare a storage type variable and use it instead of repeatedly fetching the reference in a map or an array. The effect can be quite significant. As an example, instead of repeatedly calling someMap[someIndex], save its reference like this: SomeStruct storage someStruct = someMap[someIndex] and use it.

<ins>Proof Of Concept</ins>
170: uint16 reward = uint16(rewards[winTier] / divisor);
171: if ((rewards[winTier] % divisor) != 0) {

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/LotterySetup.sol#L170-L171

78: claimedReward += claimPerDraw(drawIds[counter]);

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/ReferralSystem.sol#L78

<a href="#Summary">[GAS‑15]</a><a name="GAS&#x2011;15"> Structs can be packed into fewer storage slots

Each slot saved can avoid an extra Gsset (20000 gas) for the first setting of the struct. Subsequent reads as well as writes have smaller gas savings

<ins>Proof Of Concept</ins>
63: struct LotterySetupParams {
    /// @dev Token to be used as reward token for the lottery
    IERC20 token;
    /// @dev Parameters of the draw schedule for the lottery
    LotteryDrawSchedule drawSchedule;
    /// @dev Price to pay for playing single game (including fee)
    uint256 ticketPrice;
    /// @dev Count of numbers user picks for the ticket
    uint8 selectionSize;
    /// @dev Max number user can pick
    uint8 selectionMax;
    /// @dev Expected payout for one ticket, expressed in `rewardToken`
    uint256 expectedPayout;
    /// @dev Array of fixed rewards per each non jackpot win
    uint256[] fixedRewards;
}

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/interfaces/ILotterySetup.sol#L63-L78

Can save 1 storage slot by changing to:

63: struct LotterySetupParams {
    IERC20 token;
    uint8 selectionSize;
    uint8 selectionMax;
    LotteryDrawSchedule drawSchedule;
    uint256 ticketPrice;
    uint256 expectedPayout;
    uint256[] fixedRewards;
}

Can save an additional storage slot if ticketPrice and expectedPayout can be of size uint128 if it is unlikely to ever reach uint128.max then we can save a total of 2 storage slots by changing to:

63: struct LotterySetupParams {
    IERC20 token;
    uint8 selectionSize;
    uint8 selectionMax;
    LotteryDrawSchedule drawSchedule;
    uint128 ticketPrice;
    uint128 expectedPayout;
    uint256[] fixedRewards;
}

In addition for the following structs, these can be changed from uint256 to uint64 as it is unlikely for it to reach timestamp the max value of uint256

struct LotteryDrawSchedule {
    /// @dev First draw is scheduled to take place at this timestamp
    uint256 firstDrawScheduledAt;
    /// @dev Period for running lottery
    uint256 drawPeriod;
    /// @dev Cooldown period when users cannot register tickets for draw anymore
    uint256 drawCoolDownPeriod;
}

Can save 2 storage slots by changing to:

struct LotteryDrawSchedule {
    /// @dev First draw is scheduled to take place at this timestamp
    uint64 firstDrawScheduledAt;
    /// @dev Period for running lottery
    uint64 drawPeriod;
    /// @dev Cooldown period when users cannot register tickets for draw anymore
    uint64 drawCoolDownPeriod;
}

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/interfaces/ILotterySetup.sol#L53-L60

<a href="#Summary">[GAS‑16]</a><a name="GAS&#x2011;16"> Usage of uints/ints smaller than 32 bytes (256 bits) incurs overhead

When using elements that are smaller than 32 bytes, your contract's gas usage may be higher. This is because the EVM operates on 32 bytes at a time. Therefore, if the element is smaller than that, the EVM must use more operations in order to reduce the size of the element from 32 bytes to the desired size.

https://docs.soliditylang.org/en/v0.8.11/internals/layout_in_storage.html Each operation involving a uint8 costs an extra 22-28 gas (depending on whether the other operand is also a variable of type uint8) as compared to ones involving uint256, due to the compiler having to clear the higher bits of the memory word before operating on the uint8, as well as the associated stack operations of doing so. Use a larger size then downcast where needed

<ins>Proof Of Concept</ins>
34: uint128 public override currentDraw;

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/Lottery.sol#L34

162: uint120 _winningTicket = winningTicket[ticketInfo.drawId];

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/Lottery.sol#L162

204: uint120 _winningTicket = TicketUtils.reconstructTicket(randomNumber, selectionSize, selectionMax);

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/Lottery.sol#L204

205: uint128 drawFinalized = currentDraw++;

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/Lottery.sol#L205

273: uint128 drawId = currentDraw - LotteryMath.DRAWS_PER_YEAR;

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/Lottery.sol#L273

24: uint128 public constant DRAWS_PER_YEAR = 52;

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/LotteryMath.sol#L24

30: uint8 public immutable override selectionSize;

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/LotterySetup.sol#L30

31: uint8 public immutable override selectionMax;

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/LotterySetup.sol#L31

170: uint16 reward = uint16(rewards[winTier] / divisor);

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/LotterySetup.sol#L170

54: uint8[] memory numbers = new uint8[](selectionSize);

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/TicketUtils.sol#L54

66: uint8 currentNumber = numbers[i];

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/TicketUtils.sol#L66

94: uint120 intersection = ticket & winningTicket;

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/TicketUtils.sol#L94

97: intersection >>= 1;

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/TicketUtils.sol#L97

10: uint16 public immutable override requestConfirmations;

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/VRFv2RNSource.sol#L10

11: uint32 public immutable override callbackGasLimit;

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/VRFv2RNSource.sol#L11

71: uint8 selectionSize;

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/interfaces/ILotterySetup.sol#L71

73: uint8 selectionMax;

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/interfaces/ILotterySetup.sol#L73

14: uint128 referrerTicketCount;

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/interfaces/IReferralSystem.sol#L14

16: uint128 playerTicketCount;

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/interfaces/IReferralSystem.sol#L16

13: uint128 drawId;

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/interfaces/ITicket.sol#L13

15: uint120 combination;

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/interfaces/ITicket.sol#L15

<a href="#Summary">[GAS‑17]</a><a name="GAS&#x2011;17"> Unnecessary look up in if condition

If the || condition isn't required, the second condition will have been looked up unnecessarily.

<ins>Proof Of Concept</ins>
95: if (notEnoughRetryInvocations || notEnoughTimeReachingMaxFailedAttempts) {

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/RNSourceController.sol#L95

<a href="#Summary">[GAS‑18]</a><a name="GAS&#x2011;18"> Use solidity version 0.8.19 to gain some gas boost

Upgrade to the latest solidity version 0.8.19 to get additional gas savings. See latest release for reference: https://blog.soliditylang.org/2023/02/22/solidity-0.8.19-release-announcement/

<ins>Proof Of Concept</ins>
pragma solidity ^0.8.7;

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/VRFv2RNSource.sol#L3

pragma solidity ^0.8.7;

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/interfaces/IVRFv2RNSource.sol#L3

pragma solidity ^0.8.17;

https://github.com/code-423n4/2023-03-wenwin/tree/main/src/staking/StakedTokenLock.sol#L3

#0 - c4-judge

2023-03-12T14:29:26Z

thereksfour marked the issue as grade-a

#1 - c4-judge

2023-03-12T15:16:33Z

thereksfour marked the issue as selected for report

#2 - c4-sponsor

2023-03-14T11:38:22Z

rand0c0des marked the issue as sponsor confirmed

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax © 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter