Handle

ye0lde

Vulnerability details

Impact

Removing redundant code will save gas on deployment and execution and improve code clarity.

Proof of Concept

https://github.com/code-423n4/2021-12-perennial/blob/fd7c38823833a51ae0c6ae3856a3d93a7309c0e4/protocol/contracts/collateral/Collateral.sol#L227

https://github.com/code-423n4/2021-12-perennial/blob/fd7c38823833a51ae0c6ae3856a3d93a7309c0e4/protocol/contracts/factory/Factory.sol#L131 https://github.com/code-423n4/2021-12-perennial/blob/fd7c38823833a51ae0c6ae3856a3d93a7309c0e4/protocol/contracts/factory/Factory.sol#L140 https://github.com/code-423n4/2021-12-perennial/blob/fd7c38823833a51ae0c6ae3856a3d93a7309c0e4/protocol/contracts/factory/Factory.sol#L149 https://github.com/code-423n4/2021-12-perennial/blob/fd7c38823833a51ae0c6ae3856a3d93a7309c0e4/protocol/contracts/factory/Factory.sol#L158 https://github.com/code-423n4/2021-12-perennial/blob/fd7c38823833a51ae0c6ae3856a3d93a7309c0e4/protocol/contracts/factory/Factory.sol#L167 https://github.com/code-423n4/2021-12-perennial/blob/fd7c38823833a51ae0c6ae3856a3d93a7309c0e4/protocol/contracts/factory/Factory.sol#L176 https://github.com/code-423n4/2021-12-perennial/blob/fd7c38823833a51ae0c6ae3856a3d93a7309c0e4/protocol/contracts/factory/Factory.sol#L185 https://github.com/code-423n4/2021-12-perennial/blob/fd7c38823833a51ae0c6ae3856a3d93a7309c0e4/protocol/contracts/factory/Factory.sol#L197

https://github.com/code-423n4/2021-12-perennial/blob/fd7c38823833a51ae0c6ae3856a3d93a7309c0e4/protocol/contracts/incentivizer/Incentivizer.sol#L358 https://github.com/code-423n4/2021-12-perennial/blob/fd7c38823833a51ae0c6ae3856a3d93a7309c0e4/protocol/contracts/incentivizer/Incentivizer.sol#L368

https://github.com/code-423n4/2021-12-perennial/blob/fd7c38823833a51ae0c6ae3856a3d93a7309c0e4/protocol/contracts/oracle/ChainlinkOracle.sol#L74-L76

Tools Used

Visual Studio Code, Remix

Recommended Mitigation Steps

For example: https://github.com/code-423n4/2021-12-perennial/blob/fd7c38823833a51ae0c6ae3856a3d93a7309c0e4/protocol/contracts/oracle/ChainlinkOracle.sol#L74-L76

Change this:

    function updateMinDelay(uint256 newMinDelay) onlyOwner external {
        minDelay = newMinDelay;
        emit MinDelayUpdated(newMinDelay);
    }

to this:

    function updateMinDelay(uint256 newMinDelay) onlyOwner external {  
        emit MinDelayUpdated(minDelay = newMinDelay);  
    }

Note also that you are getting a "linter" error: Linter: Visibility modifier must be first in list of modifiers [visibility-modifier-order] for having onlyOwner before external

I don't think the change recommended will save gas, I think it will increase gas cost.

    function updateMinDelay(uint256 newMinDelay) onlyOwner external {
        minDelay = newMinDelay;
        emit MinDelayUpdated(newMinDelay);
    }

Is setting the value in Storage (SSTORE) (most likely 5000 gas for Non Zero to Non zero) and then using the memory value to emit the event EVENT COST + 3 gas (read from memory)

    function updateMinDelay(uint256 newMinDelay) onlyOwner external {  
        emit MinDelayUpdated(minDelay = newMinDelay);  
    }

Will set the value in Storage and then read from it (it may be considered hot storage so only 100 gas, but still 97 more)

There is no cost per line of code, the cost is per operation, so I think the finding there will not save gas.

Because the warden specified a linting issue, I'll mark this to non-critical. Personally would NOT recommend the sponsor to implement the finding

code-423n4 / 2021-12-perennial-findings