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

0 stars 0 forks source link

Gas Optimizations #107

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago
  1. Title: Caching .length for loop can save gas

Proof of Concept: https://github.com/code-423n4/2022-05-backd/blob/main/protocol/contracts/access/RoleManager.sol#L82 https://github.com/code-423n4/2022-05-backd/blob/main/protocol/contracts/zaps/PoolMigrationZap.sol#L39

Recommended Mitigation Steps: Change to:

    uint256 Length = roles.length;
    for (uint256 i; i < Length; i = i.uncheckedInc()) {

========================================================================

2. Title: Using != is more gas efficient

Proof of Concept: https://github.com/code-423n4/2022-05-backd/blob/main/protocol/contracts/BkdLocker.sol#L91 https://github.com/code-423n4/2022-05-backd/blob/main/protocol/contracts/BkdLocker.sol#L254

Recommended Mitigation Steps: Change to !=

    require(amount != 0, Error.INVALID_AMOUNT);

========================================================================

3. Title: Using delete statement to empty curRewardTokenData.userShares can save gas

Proof of Concept: https://github.com/code-423n4/2022-05-backd/blob/main/protocol/contracts/BkdLocker.sol#L215 https://github.com/code-423n4/2022-05-backd/blob/main/protocol/contracts/utils/Preparable.sol#L87-L88

Recommended Mitigation Steps:

    delete curRewardTokenData.userShares[msg.sender];

========================================================================

4. Title: Gas improvement on returning totalEthRequired value

Proof of Concept: https://github.com/code-423n4/2022-05-backd/blob/main/protocol/contracts/Controller.sol#L121-L130

Recommended Mitigation Steps: by set totalEthRequired in returns and delete L#123 can save gas

function getTotalEthRequiredForGas(address payer) external view override returns (uint256 totalEthRequired) { //@audit-info: set here
        // solhint-disable-previous-line ordering
    //@audit-info: remove this line
        address[] memory actions = addressProvider.allActions();
        uint256 numActions = actions.length;
        for (uint256 i = 0; i < numActions; i++) {
            totalEthRequired += IAction(actions[i]).getEthRequiredForGas(payer);
        }
    return totalEthRequired;
    }

========================================================================

5. Title: Using unchecked to calculate

Proof of Concept: https://github.com/code-423n4/2022-05-backd/blob/main/protocol/contracts/StakerVault.sol#L124

Recommended Mitigation Steps: balances[msg.sender] value was checked that it >= than amount so using unchecked can save gas:

unchecked{
    balances[msg.sender] -= amount;
}

========================================================================

6. Title: Unnecessary MSTORE timeElapsed

Proof of Concept: https://github.com/code-423n4/2022-05-backd/blob/main/protocol/contracts/tokenomics/AmmGauge.sol#L144-L149

Recommended Mitigation Steps: By passing block.timestamp - uint256(ammLastUpdated) directly to L#148 without storing it to timeElapsed can save gas without damaging readability of the code

    ammStakedIntegral += (currentRate * (block.timestamp - uint256(ammLastUpdated)).scaledDiv(totalStaked);

========================================================================

7. Title: Unnecessary MSTORE

Proof of Concept: https://github.com/code-423n4/2022-05-backd/blob/main/protocol/contracts/BkdLocker.sol#L138-L144

Recommended Mitigation Steps: instead of caching length to i. use it directly can save gas delete L#138 and replace i with `length

========================================================================

8. Title: Use allowance_ directly to substract

Proof of Concept: https://github.com/code-423n4/2022-05-backd/blob/main/protocol/contracts/StakerVault.sol#L387

Recommended Mitigation Steps:

    _allowances[src][msg.sender] = allowance_ - unstaked;

========================================================================

9. Title: Unnecessary bool var set

Proof of Concept: https://github.com/code-423n4/2022-05-backd/blob/main/protocol/contracts/tokenomics/InflationManager.sol#L412

Recommended Mitigation Steps: the default value of bool is false

========================================================================

10. Title: Using > instead >= can save gas

Proof of Concept: https://github.com/code-423n4/2022-05-backd/blob/main/protocol/contracts/tokenomics/VestedEscrow.sol#L57 https://github.com/code-423n4/2022-05-backd/blob/main/protocol/contracts/utils/Preparable.sol#L110

Recommended Mitigation Steps: 1 second difference can be ignored to validate. using > operator can save gas

    require(starttime_ > block.timestamp, "start must be future");

========================================================================

11. Title: Use custom errors rather than revert()/require() strings to save deployment gas

Proof of Concept: https://github.com/code-423n4/2022-05-backd/blob/main/protocol/contracts/tokenomics/VestedEscrowRevocable.sol#L53-L54

reference: https://blog.soliditylang.org/2021/04/21/custom-errors/

========================================================================

13. Title: Using += to increase value

Proof of Concept: https://github.com/code-423n4/2022-05-backd/blob/main/protocol/contracts/tokenomics/VestedEscrow.sol#L145

Recommended Mitigation Steps: Change to:

    totalClaimed[msg.sender] += claimable;

========================================================================

14 Title: Unnecessary MSTORE

Proof of Concept: https://github.com/code-423n4/2022-05-backd/blob/main/protocol/contracts/tokenomics/VestedEscrow.sol#L155-L156

Recommended Mitigation Steps: instead of caching to elapsed. calculate directly can save gas delete L#155

    return Math.min((locked * (_time - startTime)) / totalTime, locked);

========================================================================

GalloDaSballo commented 2 years ago

Title: Caching .length for loop can save gas

3 per instance 6

Title: Using != is more gas efficient

Saves 3 gas, only for require, solidity < 0.8.13, when using optimizer 3

Title: Using delete statement to empty curRewardTokenData.userShares can save gas

Disagree, delete is the same as setting to default value

 Title: Gas improvement on returning totalEthRequired value

I don't believe it will save gas, it's just syntactic sugar, in lack of math I can only give it 0 gas saved

Title: Using unchecked to calculate

Agree, this will save 20 gas 20

Title: Unnecessary MSTORE timeElapsed

Saves 6 gas but does make the code more complex to scan

Title: Unnecessary MSTORE

I guess, saves 6 gas, notice that the sponsor forgot to use the cached length inside the loop, that would be a more rational refactoring

 Title: Use allowance_ directly to substract

Avoiding the =- saves a SLOAD which saves 100 gas

Title: Unnecessary bool var set

Saves 3 gas

Title: Using > instead >= can save gas

I assume this will save the extra eq, hence saves 3 gas

Use custom errors rather than revert()/require() strings to save deployment gas

In lack of POC i can't give it points

Title: Using += to increase value

Doesn't save gas

Title: Unnecessary MSTORE

Saves 6 gas and makes the code pretty ugly

Total Gas Saved 153