Gas Optimizations

[code423n4]

• [S]: Suggested optimation, save a decent amount of gas without compromising readability;
• [M]: Minor optimation, the amount of gas saved is minor, change when you see fit;
• [N]: Non-preferred, the amount of gas saved is at cost of readability, only apply when gas saving is a top priority.

[M] Adding unchecked directive can save gas

For the arithmetic operations that will never over/underflow, using the unchecked directive (Solidity v0.8 has default overflow/underflow checks) can save some gas from the unnecessary internal over/underflow checks.

For example:

https://github.com/code-423n4/2022-02-redacted-cartel/blob/92c4d5810df7b9de15eae55dc7641c8b36cd799d/contracts/ThecosomataETH.sol#L118-L118

[S] Using immutable variable can save gas

https://github.com/code-423n4/2022-02-redacted-cartel/blob/92c4d5810df7b9de15eae55dc7641c8b36cd799d/contracts/TokemakBribe.sol#L28-L28

    address public bribeVault;

https://github.com/code-423n4/2022-02-redacted-cartel/blob/92c4d5810df7b9de15eae55dc7641c8b36cd799d/contracts/TokemakBribe.sol#L60-L65

    constructor(address _bribeVault) {
        require(_bribeVault != address(0), "Invalid bribeVault");
        bribeVault = _bribeVault;

        _setupRole(DEFAULT_ADMIN_ROLE, msg.sender);
    }

Considering that bribeVault will never change, changing it to immutable variable instead of storage variable can save gas.

[S] Remove redundant access control checks can save gas

https://github.com/code-423n4/2022-02-redacted-cartel/blob/92c4d5810df7b9de15eae55dc7641c8b36cd799d/contracts/TokemakBribe.sol#L125-L135

    function setProposal(address proposal, uint256 deadline)
        public
        onlyAuthorized
    {
        require(proposal != address(0), "Invalid proposal");
        require(deadline >= block.timestamp, "Deadline must be in the future");

        proposalDeadlines[proposal] = deadline;

        emit SetProposal(proposal, deadline, _round);
    }

https://github.com/code-423n4/2022-02-redacted-cartel/blob/92c4d5810df7b9de15eae55dc7641c8b36cd799d/contracts/TokemakBribe.sol#L142-L157

    function setProposals(
        address[] calldata proposals,
        uint256[] calldata deadlines
    ) external onlyAuthorized {
        require(proposals.length > 0, "Need at least 1 proposal");
        require(
            proposals.length == deadlines.length,
            "Must be equal # of proposals and deadlines"
        );

        for (uint256 i = 0; i < proposals.length; i += 1) {
            setProposal(proposals[i], deadlines[i]);
        }

        emit SetProposals(proposals, deadlines, _round);
    }

setProposal() already got onlyAuthorized check, and setProposals() will check it again multiple times.

Consider creating _setProposal() private function without access control and call it inside the public functions.

Recommendation

Change to:

    function _setProposal(address proposal, uint256 deadline)
        private
    {
        require(proposal != address(0), "Invalid proposal");
        require(deadline >= block.timestamp, "Deadline must be in the future");

        proposalDeadlines[proposal] = deadline;
    }

    /**
        @notice Set a single proposal
        @param  proposal  addresss Proposal address
        @param  deadline  uint256  Proposal deadline
     */
    function setProposal(address proposal, uint256 deadline)
        public
        onlyAuthorized
    {
        _setProposal(proposal, deadline);
        emit SetProposal(proposal, deadline, _round);
    }

    /**
        @notice Set multiple proposals
        @param  proposals  address[]  Proposal addresses
        @param  deadlines  uint256[]  Proposal deadlines
     */
    function setProposals(
        address[] calldata proposals,
        uint256[] calldata deadlines
    ) external onlyAuthorized {
        require(proposals.length > 0, "Need at least 1 proposal");
        require(
            proposals.length == deadlines.length,
            "Must be equal # of proposals and deadlines"
        );

        for (uint256 i = 0; i < proposals.length; i += 1) {
            _setProposal(proposals[i], deadlines[i]);
        }

        emit SetProposals(proposals, deadlines, _round);
    }

[S] Validation can be done earlier to save gas

Check if ethLiquidity > 0 && btrflyLiquidity > 0 earlier can avoid unnecessary external call (IRedactedTreasury(TREASURY).manage(WETH, ethLiquidity);) when this check failed.

https://github.com/code-423n4/2022-02-redacted-cartel/blob/92c4d5810df7b9de15eae55dc7641c8b36cd799d/contracts/ThecosomataETH.sol#L124-L155

    function performUpkeep() external onlyOwner {
        require(checkUpkeep(), "Invalid upkeep state");

        uint256 btrfly = IBTRFLY(BTRFLY).balanceOf(address(this));
        uint256 ethAmount = calculateAmountRequiredForLP(btrfly, true);
        uint256 ethCap = IERC20(WETH).balanceOf(TREASURY);
        uint256 ethLiquidity = ethCap > ethAmount ? ethAmount : ethCap;

        // Use BTRFLY balance if remaining capacity is enough, otherwise, calculate BTRFLY amount
        uint256 btrflyLiquidity = ethCap > ethAmount
            ? btrfly
            : calculateAmountRequiredForLP(ethLiquidity, false);

        IRedactedTreasury(TREASURY).manage(WETH, ethLiquidity);

        // Only complete upkeep only on sufficient amounts
        require(ethLiquidity > 0 && btrflyLiquidity > 0, "Insufficient amounts");
        // ...
    }

Recommendation

Change to:

    function performUpkeep() external onlyOwner {
        require(checkUpkeep(), "Invalid upkeep state");

        uint256 btrfly = IBTRFLY(BTRFLY).balanceOf(address(this));
        uint256 ethAmount = calculateAmountRequiredForLP(btrfly, true);
        uint256 ethCap = IERC20(WETH).balanceOf(TREASURY);
        uint256 ethLiquidity = ethCap > ethAmount ? ethAmount : ethCap;

        // Use BTRFLY balance if remaining capacity is enough, otherwise, calculate BTRFLY amount
        uint256 btrflyLiquidity = ethCap > ethAmount
            ? btrfly
            : calculateAmountRequiredForLP(ethLiquidity, false);

        // Only complete upkeep only on sufficient amounts
        require(ethLiquidity > 0 && btrflyLiquidity > 0, "Insufficient amounts");

        IRedactedTreasury(TREASURY).manage(WETH, ethLiquidity);

        // ...
    }

[M] type(uint256).max is more gas efficient than 2**256 - 1

https://github.com/code-423n4/2022-02-redacted-cartel/blob/92c4d5810df7b9de15eae55dc7641c8b36cd799d/contracts/ThecosomataETH.sol#L68-L69

[M] 10e18 is more gas efficient than 10**18

https://github.com/code-423n4/2022-02-redacted-cartel/blob/92c4d5810df7b9de15eae55dc7641c8b36cd799d/contracts/ThecosomataETH.sol#L102-L108

[S] Cache array length in for loops can save gas

Reading array length at each iteration of the loop takes 6 gas (3 for mload and 3 to place memory_offset) in the stack.

Caching the array length in the stack saves around 3 gas per iteration.

Instances include:

https://github.com/code-423n4/2022-02-redacted-cartel/blob/92c4d5810df7b9de15eae55dc7641c8b36cd799d/contracts/TokemakBribe.sol#L147-L152

https://github.com/code-423n4/2022-02-redacted-cartel/blob/92c4d5810df7b9de15eae55dc7641c8b36cd799d/contracts/BribeVault.sol#L261-L275

https://github.com/code-423n4/2022-02-redacted-cartel/blob/92c4d5810df7b9de15eae55dc7641c8b36cd799d/contracts/RewardDistributor.sol#L80-L82

[S] Avoid unnecessary storage read can save gas

https://github.com/code-423n4/2022-02-redacted-cartel/blob/92c4d5810df7b9de15eae55dc7641c8b36cd799d/contracts/BribeVault.sol#L213-L248

    function depositBribe(
        bytes32 bribeIdentifier,
        bytes32 rewardIdentifier,
        address briber
    ) external payable onlyRole(DEPOSITOR_ROLE) {
        require(bribeIdentifier.length > 0, "Invalid bribeIdentifier");
        require(rewardIdentifier.length > 0, "Invalid rewardIdentifier");
        require(briber != address(0), "Invalid briber");
        require(msg.value > 0, "Value must be greater than 0");

        Bribe storage b = bribes[bribeIdentifier];
        address currentToken = b.token;
        require(
            // For native tokens, the token address is set to this contract to prevent
            // overwriting storage - the address can be anything but address(this) safer
            currentToken == address(0) || currentToken == address(this),
            "Cannot change token"
        );

        b.amount += msg.value; // Allow bribers to increase bribe

        // Only set the token address and update the reward-to-bribe mapping if not yet set
        if (currentToken == address(0)) {
            b.token = address(this);
            rewardToBribes[rewardIdentifier].push(bribeIdentifier);
        }

        emit DepositBribe(
            bribeIdentifier,
            rewardIdentifier,
            b.token,
            msg.value,
            b.amount,
            briber
        );
    }

Based on L224L230, L235L236, we know that b.token == address(this), therefore at L243 b.token can be replaced with address(this).

Use address(this) directly can avoid unnecessary storage read of b.token and save some gas.

Recommendation

Replace:

emit DepositBribe(
    bribeIdentifier,
    rewardIdentifier,
    b.token,
    msg.value,
    b.amount,
    briber
);

with:

emit DepositBribe(
    bribeIdentifier,
    rewardIdentifier,
    address(this),
    msg.value,
    b.amount,
    briber
);

[drahrealm]

Some new gas optimization tricks confirmed 👍

[GalloDaSballo]

Submission is really good

Adding the exact gas savings would be the cherry on top

Additionally adding a list of all the places in which to apply the optimization would have made this the best finding.

Pretty good

[GalloDaSballo]

Best submission 8/10

To improve:

• Actual Gas Savings math (sort findings by impact)
• List of all spots to fix (So the sponsor can implement instead of it being a puzzle)