code423n4 commented 2 years ago

G01 Custom Errors

As your using a solidity version greater 0.8.4 you can replace revert strings with custom errors. This will save in deployment costs and runtime costs. I ran a test in remix comparing a revert string vs custom errors and found that replacing a single revert string with a custom error saved 12,404 gas in deployment cost and 86 gas on each function call.

contract Test {
    uint256 a;
    function check() external {
        require(a != 0, "check failed");
    }
}   (Deployment cost: 114,703, Cost on Function call: 23,392)
vs 
contract Test {
    uint256 a;
    error checkFailed();
    function check() external {
        if (a != 0) revert checkFailed();
    }
}   (Deployment cost: 102,299, Cost on Function call: 23,306)

There are 131 revert strings throughout the in scope files that can be replaced with custom errors:

G02 Long Revert Strings

If you opt not to use custom errors keeping revert strings <= 32 bytes in length will save gas. I ran a test in remix and found the savings for a single short revert string vs long string to be 9,377 gas in deployment cost and 18 gas on function call.

contract Test {
    uint256 a;
    function check() external {
        require(a != 0, "short error message"); 
        (Deployment cost: 114,799, Cost on function call: 23,392)   
        vs 
        require(a != 0, "A longer Error Message over 32 bytes in     
        length"); 
        (Deployment cost: 124,176, Cost on function call: 23,410)   
    }
}

I recommend shortenning the following revert strings to <= 32 bytes in length: TimelockControllerEmergency.sol#L229-L230 TimelockControllerEmergency.sol#L243-L244 TimelockControllerEmergency.sol#L256 TimelockControllerEmergency.sol#L319-L320 TimelockControllerEmergency.sol#L334-L335 TimelockControllerEmergency.sol#L342 TimelockControllerEmergency.sol#L359 TimelockControllerEmergency.sol#L375

G03 Public Functions that can be External

The following functions are currently set to public but are not called anywhere in their contract and can be switched to external.

NestedFactory.sol#L114 OwnableProxyDelegation.sol#L50 OwnableProxyDelegation.sol#L56 OwnerProxy.sol#L16 TimelockControllerEmergency.sol#L199-L206 TimelockControllerEmergency.sol#L221-L228 TimelockControllerEmergency.sol#L255 TimelockControllerEmergency.sol#L274-L280 TimelockControllerEmergency.sol#L295-L299 TimelockControllerEmergency.sol#L312-L318

G04 For Loop Optimisations

When incrementing i in for loops there is no chance of overflow so unchecked can be used to save gas. I ran a simple test in remix and found deployment savings of 31,653 gas and on each function call saved ~141 gas per iteration.

contract Test {
    function loopTest() external {
        for (uint256 i; i < 1; ++i) {
        Deployment Cost: 125,637, Cost on function call: 24,601
        vs
        for (uint256 i; i < 1; ) {
        // for loop body
        unchecked { ++i }
        Deployment Cost: 93,984, Cost on function call: 24,460
        }
    }
}

In for loops pre increments can also be used to save a small amount of gas per iteraition. I ran a test in remix using a for loop and found the deployment savings of 497 gas and ~5 gas per iteration.

contract Test {
    function loopTest() external {
        for (uint256 i; i < 1; i++) {
        (Deployment cost: 118,408, Cost on function call: 24,532)
        vs
        for (uint256 i; i < 1; ++i) {
        (Deployment cost: 117,911, Cost on function call: 24,527)
        }
    }
}

Instances where unchecked and pre Increments can be implemented in for loops:

G05 && in Require Functions

If optimising for running costs over deployment costs you can seperate && in require functions into 2 parts. I ran a basic test in remix and it cost an extra 234 gas to deploy but will save ~9 gas everytime the require function is called.

contract Test {
    uint256 a = 0;
    uint256 b = 1;

    function test() external {
        require(a == 0 && b > a) 
        (Deployment cost: 123,291, Cost on function call: 29,371)
        vs
        require(a == 0);
        require(b > a);
        (Deployment cost: 123,525, Cost on function call: 29,362)
    }
}

BeefyVaultOperator.sol#L54 BeefyZapBiswapLPVaultOperator.sol#L64-L65 BeefyZapUniswapLPVaultOperator.sol#L64-L65 ParaswapOperator.sol#L16