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

6 stars 4 forks source link

Gas Optimizations #141

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Gas saving

The functions addFeeHandler and removeFeeHandler will always return true, consider remove the return true to save gas, also i think it will be a good practice that they emit an event.

File: https://github.com/code-423n4/2022-04-backd/blob/2c5248d6579254cfffc40a0bf5914531f3d5da02/backd/contracts/AddressProvider.sol#L63-L73

Current code:

    function addFeeHandler(address feeHandler) external onlyGovernance returns (bool) {
        require(!_whiteListedFeeHandlers.contains(feeHandler), Error.ADDRESS_WHITELISTED);
        _whiteListedFeeHandlers.add(feeHandler);
        return true;
    }

    function removeFeeHandler(address feeHandler) external onlyGovernance returns (bool) {
        require(_whiteListedFeeHandlers.contains(feeHandler), Error.ADDRESS_NOT_WHITELISTED);
        _whiteListedFeeHandlers.remove(feeHandler);
        return true;
    }

Recommendation;

    event AddFeeHandler(address feeHandler);
    event RemoveFeeHandler(address feeHandler);

    function addFeeHandler(address feeHandler) external onlyGovernance {
        require(!_whiteListedFeeHandlers.contains(feeHandler), Error.ADDRESS_WHITELISTED);
        _whiteListedFeeHandlers.add(feeHandler);
       emit AddFeeHandler(feeHandler);
    }

    function removeFeeHandler(address feeHandler) external onlyGovernance {
        require(_whiteListedFeeHandlers.contains(feeHandler), Error.ADDRESS_NOT_WHITELISTED);
        _whiteListedFeeHandlers.remove(feeHandler);
       emit RemoveFeeHandler(feeHandler);
    }
chase-manning commented 2 years ago

This is not gas saving, this is a change to functionality.