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

2 stars 0 forks source link

Gas Optimizations #231

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

[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.

[S-1] Cally.sol#getPremium() Implementation can be simpler and save some gas

https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L394-L397

    function getPremium(uint256 vaultId) public view returns (uint256 premium) {
        Vault memory vault = _vaults[vaultId];
        return premiumOptions[vault.premiumIndex];
    }

Recommendation

Change to:

    function getPremium(uint256 vaultId) public view returns (uint256) {
        return premiumOptions[_vaults[vaultId].premiumIndex];
    }

[M-2] Cache storage varibales can save gas

https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L90-L92

uint256[] public premiumOptions = [0.01 ether, 0.025 ether, 0.05 ether, 0.075 ether, 0.1 ether, 0.25 ether, 0.5 ether, 0.75 ether, 1.0 ether, 2.5 ether, 5.0 ether, 7.5 ether, 10 ether, 25 ether, 50 ether, 75 ether, 100 ether];
    // prettier-ignore
uint256[] public strikeOptions = [1 ether, 2 ether, 3 ether, 5 ether, 8 ether, 13 ether, 21 ether, 34 ether, 55 ether, 89 ether, 144 ether, 233 ether, 377 ether, 610 ether, 987 ether, 1597 ether, 2584 ether, 4181 ether, 6765 ether];

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

require(premiumIndex < premiumOptions.length, "Invalid premium index");

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

        require(dutchAuctionStartingStrikeIndex < strikeOptions.length, "Invalid strike index");
        require(dutchAuctionReserveStrike < strikeOptions[dutchAuctionStartingStrikeIndex], "Reserve strike too small");

strikeOptions and premiumOptions can be cached to save gas from unnecessary SLOAD.

[S-3] Cally.sol#getDutchAuctionStrike() Implementation can be simpler and save some gas

https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L417-L422

        uint256 delta = auctionEndTimestamp > block.timestamp ? auctionEndTimestamp - block.timestamp : 0;
        uint256 progress = (1e18 * delta) / AUCTION_DURATION;
        uint256 auctionStrike = (progress * progress * startingStrike) / (1e18 * 1e18);

        // max(auctionStrike, reserveStrike)
        strike = auctionStrike > reserveStrike ? auctionStrike : reserveStrike;

Recommendation

Change to:

        uint256 delta = auctionEndTimestamp > block.timestamp ? auctionEndTimestamp - block.timestamp : 0;

        if (delta == 0) return reserveStrike;

        uint256 progress = (1e18 * delta) / AUCTION_DURATION;
        uint256 auctionStrike = (progress * progress * startingStrike) / (1e18 * 1e18);

        // max(auctionStrike, reserveStrike)
        strike = auctionStrike > reserveStrike ? auctionStrike : reserveStrike;

When delta == 0, lots of arithmetic operations can be avoided.

[M-4] Setting uint256 variables to 0 is redundant

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

    uint256 public feeRate = 0;
    uint256 public protocolUnclaimedFees = 0;

https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L282

        uint256 fee = 0;

Setting uint256 variables to 0 is redundant as they default to 0.

[M-5] Avoid unnecessary storage reads can save gas

For the storage variables that will be accessed multiple times, cache and read from the stack can save ~100 gas from each extra read (SLOAD after Berlin).

In Cally.sol#createVault(), vaultIndex is read twice.

https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L187-L190

        // vault index should always be odd
        vaultIndex += 2;
        vaultId = vaultIndex;
        _vaults[vaultId] = vault;

Recommendation

Change to:

        // vault index should always be odd
        vaultIndex = vaultId = vaultIndex + 2;
        _vaults[vaultId] = vault;

[M-6] Remove unnecessary variable can make the code simpler and save some gas

https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L226-L228

        // check option associated with the vault has expired
        uint32 auctionStartTimestamp = vault.currentExpiration;
        require(block.timestamp >= auctionStartTimestamp, "Auction not started");

auctionStartTimestamp is unnecessary as it's being used only once.

Recommendation

Change to:

        // check option associated with the vault has expired
        require(block.timestamp >= vault.currentExpiration, "Auction not started");
outdoteth commented 2 years ago

high quality report