Forgotten Runes Warrior Guild contest - CertoraInc's results

16,000 Warrior NFTs sold in a phased Dutch Auction.

General Information

Platform: Code4rena

Start Date: 03/05/2022

Pot Size: $30,000 USDC

Total HM: 6

Participants: 93

Period: 3 days

Judge: gzeon

Id: 118

League: ETH

Forgotten Runes

Findings Distribution

Researcher Performance

Rank: 65/93

Findings: 2

Award: $45.77

🌟 Selected for report: 0

🚀 Solo Findings: 0

Team actions don't have any limitations. This can look a little bit suspicious, the users will have to trust the protocol owners and in my opinion it'll be better to add limitations to this actions.

Gas Optimizations

  • variables in solidity are initialized automatically to their default value, and it actually costs more gas to initialize them manually, so the numMinted variable in the ForgottenRunesWarriorsGuild contract shouldn't be initialized

    uint256 public numMinted;
    // uint256 public numMinted = 0; // costs more gas
  • Use unchecked in the bidSummon function - we know for sure that the first expression won't overflow because currentPrice * numWarriors <= startingPrice * 20 < 2**256 - 1, and the second calculations won't overflow because daNumMinted[msg.sender] + numWarriors <= numSold + numWarriors <= max supply < 2**256 - 1

    require(
        msg.value >= (currentPrice * numWarriors),
        'Ether value sent is not sufficient'
    );
    // ...
    daNumMinted[msg.sender] += numWarriors;
    numSold += numWarriors;
  • Use unchecked in the currentDaPrice function - we know that stepDeduction <= startPrice so startPrice - stepDeduction won't underflow

    if (stepDeduction > startPrice) {
        return lowestPrice;
    }
    uint256 currentPrice = startPrice - stepDeduction;
  • Loops can be optimized in several ways. Let's take for example the loop in the bidSummon, teamSummon and in the publicSummon functions

    for (uint256 i = 0; i < numWarriors; i++) {
        _mint(msg.sender);
    }

    We can do multiple things here:

    1. Variables in solidity are already initialized to their default value, and initializing them to the same value actually costs more gas. So for example in the loop above, the code can be optimized using uint i; instead of uint i = 0;.
    2. Use ++i instead of i++ to save some gas spent in every iteration.
    3. Use unchecked when incrementing i So after all these changes, the code will look something like this:
    for (uint256 i; i < numWarriors; ) {
        _mint(msg.sender);
        unchecked { ++i; }
    }

    There is another loop in the issueRefunds function you can apply similar changes to in order to optimize it.

  • Gas optimizations of the issueRefunds function (and some helper functions it uses)

    I optimized this function with multiple optimizations, so I'll describe them in steps, so it'll be easier for you to review each optimization alone and use the ones you want.

    This is the code of the original function with the helper functions it uses:

    function issueRefunds(uint256 startIdx, uint256 endIdx)
        public
        onlyOwner
        nonReentrant
    {
        for (uint256 i = startIdx; i < endIdx + 1; i++) {
            _refundAddress(daMinters[i]);
        }
    }
    
    function _refundAddress(address minter) private {
        uint256 owed = refundOwed(minter);
        if (owed > 0) {
            daAmountRefunded[minter] += owed;
            _safeTransferETHWithFallback(minter, owed);
        }
    }
    
    function refundOwed(address minter) public view returns (uint256) {
        uint256 totalCostOfMints = finalPrice * daNumMinted[minter];
        uint256 refundsPaidAlready = daAmountRefunded[minter];
        return daAmountPaid[minter] - totalCostOfMints - refundsPaidAlready;
    }
    
    /**
     * @notice Transfer ETH. If the ETH transfer fails, wrap the ETH and try send it as WETH.
     * @param to account who to send the ETH or WETH to
     * @param amount uint256 how much ETH or WETH to send
     */
    function _safeTransferETHWithFallback(address to, uint256 amount) internal {
        if (!_safeTransferETH(to, amount)) {
            IWETH(weth).deposit{value: amount}();
            IERC20(weth).transfer(to, amount);
        }
    }
    
    /**
     * @notice Transfer ETH and return the success status.
     * @dev This function only forwards 30,000 gas to the callee.
     * @param to account who to send the ETH to
     * @param value uint256 how much ETH to send
     */
    function _safeTransferETH(address to, uint256 value)
        internal
        returns (bool)
    {
        (bool success, ) = to.call{value: value, gas: 30_000}(new bytes(0));
        return success;
    }

    The starting gas cost from the report is 69745 - 143756 (Min = 69745, Max = 143756).

    1. The first optimization is the loop optimization that I described before. These optimizations reduce the cost to 69531 - 143542.

    2. The second optimization is that instead of using the daAmountRefunded mapping, you can simply subtract from the daAmountPaid value of the user.

      This optimization will make the contract use less storage and make the gas cost of the function a lot cheaper, because the reduction in the storage usage and because the refundOwed function will return daAmountPaid[minter] - totalCostOfMints instead of daAmountPaid[minter] - totalCostOfMints - refundsPaidAlready which is a simpler calculation.

      This optimization reduces the gas cost to 50181 - 124192, which is a big change compared to the previous cost.

    3. Inline the safeTransferEthWithFallback and safeTransferEth function, which means write their code inside the function instead of calling the functions.

      This reduces the cost to 50098 - 124097.

    4. Use unchecked in the _refundAddress function - the calculations will be something like

      uint newTotalPaid = finalPrice * daNumMinted[minter];
      uint owed = totalPaid - newTotalPaid;

      and you can use unchecked to make them cheaper (we know it won't overflow or underflow because we know that finalPrice * daNumMinted[minter] <= newTotalPaid). This reduces the cost to 50009 - 124008.

    5. Replace the bytes(0) with "" in the low level call.

    6. Inline the _refundAddress and refundOwed functions, and also optimize the code. Some optimization can be done there because the inlined code uses the same (storage) values that can be saved in order to use less storage.


    The new code will look something like this:

    function issueRefunds(uint256 startIdx, uint256 endIdx)
        public
        onlyOwner
        nonReentrant
    {
        unchecked {
            for (uint256 i = startIdx; i <= endIdx; ) {
                // _refundAddress(daMinters[i]);
                address minter = daMinters[i];
                uint totalPaid = daAmountPaid[minter];
                // uint256 owed = refundOwed(minter);
                uint newTotalPaid = finalPrice * daNumMinted[minter];
                uint owed = totalPaid - newTotalPaid;
                if (owed > 0) {
                    // daAmountRefunded[minter] += owed;
                    daAmountPaid[minter] = newTotalPaid;
                    // _safeTransferETHWithFallback(, owed);
                    (bool success, ) = minter.call{value: owed, gas: 30_000}("");
                    if (!success) {
                        IWETH(weth).deposit{value: owed}();
                        IERC20(weth).transfer(minter, owed);
                    }
                }
                ++i;
            }
        }
    }

    The final cost of this implementation is 49485 - 123444, which is ~29% improvement on the minimum cost, and ~14% on the maximum cost.

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