sherlock-audit / 2023-10-looksrare-judging

6 stars 6 forks source link

cheatc0d3 - setMintPeriod Function can be more optimized #110

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

cheatc0d3

medium

setMintPeriod Function can be more optimized

Summary

Cache mintStart and mintEnd in memory at the start of the function to reduce storage reads. https://github.com/sherlock-audit/2023-10-looksrare/blob/main/contracts-infiltration/contracts/Infiltration.sol#L416

Vulnerability Detail

I've introduced _mintStart and _mintEnd as memory variables to cache the storage values of mintStart and mintEnd. I've removed the conditional in the emit statement since the function will revert if newMintStart is invalid, ensuring that the correct values are always emitted.

Impact

The function can be optimized to save more gas.

Code Snippet

function setMintPeriod(uint40 newMintStart, uint40 newMintEnd) external onlyOwner {
    if (newMintStart >= newMintEnd) {
        revert InvalidMintPeriod();
    }

    if (newMintStart != 0) {
        if (block.timestamp > newMintStart) {
            revert MintStartIsInThePast();
        }

        uint256 currentMintStart = mintStart;
        if (currentMintStart != 0) {
            if (block.timestamp >= currentMintStart) {
                revert MintAlreadyStarted();
            }
        }

        mintStart = newMintStart;
    }

    if (block.timestamp > newMintEnd || newMintEnd < mintEnd) {
        revert MintCanOnlyBeExtended();
    }

    mintEnd = newMintEnd;

    emit MintPeriodUpdated(newMintStart == 0 ? mintStart : newMintStart, newMintEnd);
}

Optimized code:

function setMintPeriod(uint40 newMintStart, uint40 newMintEnd) external onlyOwner {
    // Cache mintStart and mintEnd in memory
    uint40 _mintStart = mintStart;
    uint40 _mintEnd = mintEnd;

    if (newMintStart >= newMintEnd) {
        revert InvalidMintPeriod();
    }

    if (newMintStart != 0) {
        if (block.timestamp > newMintStart) {
            revert MintStartIsInThePast();
        }

        // Use the cached value
        if (_mintStart != 0) {
            if (block.timestamp >= _mintStart) {
                revert MintAlreadyStarted();
            }
        }

        // Update the storage value
        mintStart = newMintStart;
    }

    // Use the cached value and update the condition
    if (block.timestamp > newMintEnd || newMintEnd < _mintEnd) {
        revert MintCanOnlyBeExtended();
    }

    // Update the storage value
    mintEnd = newMintEnd;

    // Simplify the event emission
    emit MintPeriodUpdated(newMintStart, newMintEnd);
}

Tool used

Manual Review

Recommendation

Cache mintStart and mintEnd in memory at the start of the function to reduce storage reads.

Duplicate of #109