Gas Optimizations - Githubissues

GAS

1. Title: Var set with default value

Occurrences: https://github.com/code-423n4/2022-05-cally/blob/main/contracts/src/Cally.sol#L94-L95

By just declare var without set the value (if value is default) is way efficient

    uint256 public feeRate;
    uint256 public protocolUnclaimedFees;

2. Title: Using uint256 for uint inside struct

https://github.com/code-423n4/2022-05-cally/blob/main/contracts/src/Cally.sol#L76-L79

Using uint256 is way more efficient than uint8 or uint 32 inside the struct. The reason for this is that the EVM reads 32 bytes at a time and will have to do some operations to make the data it is reading go down to 8 bits (1 byte).

Source: https://dev.to/javier123454321/solidity-gas-optimizations-pt-3-packing-structs-23f4

RECOMMENDED MITIGATION STEP

struct Vault {
        uint256 tokenIdOrAmount;
        uint256 premiumIndex; // indexes into `premiumOptions`
        uint256 durationDays; // days
        uint256 dutchAuctionStartingStrikeIndex; // indexes into `strikeOptions`
        uint256 currentExpiration;
        address token; //@audit-info: Move address here to save 1 slot
        bool isExercised;
        bool isWithdrawing;
        TokenType tokenType;
        uint256 currentStrike;
        uint256 dutchAuctionReserveStrike;
    }

3. Title: Using ! operator to check value is false

https://github.com/code-423n4/2022-05-cally/blob/main/contracts/src/Cally.sol#L217-L220 https://github.com/code-423n4/2022-05-cally/blob/main/contracts/src/Cally.sol#L328

Using ! operator is way more effective than using == to validate bool var value is false

RECOMMENDED MITIGATION STEP

require(!vault.isExercised, "Vault already exercised");

It can save 15 execution gas cost

4. Title: Using storage pointer to store struct var

Occurrences: https://github.com/code-423n4/2022-05-cally/blob/main/contracts/src/Cally.sol#L208 https://github.com/code-423n4/2022-05-cally/blob/main/contracts/src/Cally.sol#L266 https://github.com/code-423n4/2022-05-cally/blob/main/contracts/src/Cally.sol#L395 https://github.com/code-423n4/2022-05-cally/blob/main/contracts/src/Cally.sol#L325

Reading directly from storage than storing it to memory can save a lot of gas fee (especially if there is a lot data were stored in the struct) RECOMMENDED MITIGATION STEP

        Vault storage vault = _vaults[vaultId]; // @audit-info: Change memory to storage

5. Title: Using ** operator

https://github.com/code-423n4/2022-05-cally/blob/main/contracts/src/Cally.sol#L419

Instead doing var * var (which is doing 2 times MLOAD) using var**2 is way more effective and increasing readability of the code Change to:

    //@audit-info: has no effect for gas opt by using it on 1e18, but still recommending doing so
    uint256 auctionStrike = (progress**2 * startingStrike) / (1e18**2);

6. Title: Check the vaultId is odd earlier

https://github.com/code-423n4/2022-05-cally/blob/main/contracts/src/Cally.sol#L211

Before initializing vault (currently at L208) which cost a lot of gas fee. Check it == odd number is way more effective (it doesn't need to read the data from _vaults[vaultId]) Change to:

    function buyOption(uint256 vaultId) external payable returns (uint256 optionId) {
    // vaultId should always be odd
        require(vaultId % 2 != 0, "Not vault type");        

    Vault memory vault = _vaults[vaultId]; //@audit-info: Replace the location of L208 & L211

7. Title: Using != operator

Occurrences: https://github.com/code-423n4/2022-05-cally/blob/main/contracts/src/Cally.sol#L170 https://github.com/code-423n4/2022-05-cally/blob/main/contracts/src/Cally.sol#L283

Using != operator is way more effective than > to validate that the var's value is not zero

8. Title: Using value to validate fixed array.length

https://github.com/code-423n4/2022-05-cally/blob/main/contracts/src/Cally.sol#L167-L168

Instead of SLOADing from storage, put the array.length directly if the array.length is fixed is cost less gas Change to:

        require(premiumIndex < 17, "Invalid premium index");
        require(dutchAuctionStartingStrikeIndex < 19, "Invalid strike index");

The error string has made the code is still clear and readable

9. Title: Doing MLOAD auctionStartTimestamp instead SLOAD

https://github.com/code-423n4/2022-05-cally/blob/main/contracts/src/Cally.sol#L233

vault.currentExpiration was MSTOREd at L227. Using it's memory var for getDutchAuctionStrike() argument can save gas by avoiding doing SLOAD Change to:

        vault.currentStrike = getDutchAuctionStrike(
            strikeOptions[vault.dutchAuctionStartingStrikeIndex],
            auctionStartTimestamp + AUCTION_DURATION, //@audit-info: change made at this line
            vault.dutchAuctionReserveStrike
        );

10. Title: Efficiency in getDutchAuctionStrike()

https://github.com/code-423n4/2022-05-cally/blob/main/contracts/src/Cally.sol#L406-L423

If auctionEndTimestamp > block.timestamp is false, the return value will always 0. It's gas efficient to just skip all the math operation if return value is expected 0.

Change to:

    function getDutchAuctionStrike(
        uint256 startingStrike,
        uint32 auctionEndTimestamp,
        uint256 reserveStrike
    ) public view returns (uint256 strike) {
        /*
            delta = max(auctionEnd - currentTimestamp, 0)
            progress = delta / auctionDuration
            auctionStrike = progress^2 * startingStrike
            strike = max(auctionStrike, reserveStrike)
        */
    if(auctionEndTimestamp > block.timestamp){ //@audit-info: Condition checked here 
            uint256 delta = auctionEndTimestamp - block.timestamp; //@audit-info: Changes made here
            uint256 progress = (1e18 * delta) / AUCTION_DURATION;
            uint256 auctionStrike = (progress * progress * startingStrike) / (1e18 * 1e18);

            // max(auctionStrike, reserveStrike)
            strike = auctionStrike > reserveStrike ? auctionStrike : reserveStrike;
    }
    //@audit-info: strike will has default (0) value if it's not set
    }

11. Title: Using _ownerOf instead ownerOf()

Occurrences: https://github.com/code-423n4/2022-05-cally/blob/main/contracts/src/Cally.sol#L214 https://github.com/code-423n4/2022-05-cally/blob/main/contracts/src/Cally.sol#L263

ownerOf() is a function which reverting the transaction if vaultId isn't exist. By using _ownerOf[vaultId] inside the require() or calling ownerOf(vaultId) without require can save gas

Change to:

        require(_ownerOf[vaultId] != address(0), "Vault does not exist");

Or:

    ownerOf(vaultId);

code-423n4 / 2022-05-cally-findings

Gas Optimizations #290

GAS