code-423n4 / 2022-02-aave-lens-findings

0 stars 0 forks source link

Gas Optimizations #79

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Use uint256 instead of bool

Use uint(1) instead of bool(true) to save gas by avoiding masking ops https://github.com/code-423n4/2022-02-aave-lens/blob/c1d2de2b0609b7d2734ada2ce45c91a73cc54dd9/contracts/core/storage/LensHubStorage.sol#L59

    mapping(address => bool) internal _profileCreatorWhitelisted;
    mapping(address => bool) internal _followModuleWhitelisted;
    mapping(address => bool) internal _collectModuleWhitelisted;
    mapping(address => bool) internal _referenceModuleWhitelisted;

Unchecked increment

It is virtually impossible for these counter to overflow https://github.com/code-423n4/2022-02-aave-lens/blob/c1d2de2b0609b7d2734ada2ce45c91a73cc54dd9/contracts/core/LensHub.sol#L148

        uint256 profileId = ++_profileCounter;

https://github.com/code-423n4/2022-02-aave-lens/blob/c1d2de2b0609b7d2734ada2ce45c91a73cc54dd9/contracts/core/CollectNFT.sol#L51

        uint256 tokenId = ++_tokenIdCounter;

https://github.com/code-423n4/2022-02-aave-lens/blob/c1d2de2b0609b7d2734ada2ce45c91a73cc54dd9/contracts/core/FollowNFT.sol#L58

        uint256 tokenId = ++_tokenIdCounter;

_validateHandle gas optimization

'.' must be < '0' https://github.com/code-423n4/2022-02-aave-lens/blob/c1d2de2b0609b7d2734ada2ce45c91a73cc54dd9/contracts/libraries/PublishingLogic.sol#L398

    function _validateHandle(string calldata handle) private pure {
        bytes memory byteHandle = bytes(handle);
        if (byteHandle.length == 0 || byteHandle.length > Constants.MAX_HANDLE_LENGTH)
            revert Errors.HandleLengthInvalid();

        for (uint256 i = 0; i < byteHandle.length; ++i) {
            if (
                ((byteHandle[i] < '0' && byteHandle[i] != '.') ||
                    byteHandle[i] > 'z' ||
                    (byteHandle[i] > '9' && byteHandle[i] < 'a'))
            ) revert Errors.HandleContainsInvalidCharacters();
        }
    }

createProfile gas optimization

https://github.com/code-423n4/2022-02-aave-lens/blob/c1d2de2b0609b7d2734ada2ce45c91a73cc54dd9/contracts/libraries/PublishingLogic.sol#L57

        bytes memory followModuleReturnData;
        if (vars.followModule != address(0)) {
            _profileById[profileId].followModule = vars.followModule;
            followModuleReturnData = _initFollowModule(
                profileId,
                vars.followModule,
                vars.followModuleData,
                _followModuleWhitelisted
            );
        }else{
            followModuleReturnData = new bytes(0);
        }

        _emitProfileCreated(profileId, vars, followModuleReturnData);
    }
Zer0dot commented 2 years ago

So this is fairly low quality in that there is basically no explanation and it doesn't follow the required format, but anyway the unchecked increment thing is true, I'm looking at how best to implement that! Uint256 instead of bool is absolutely out of the question because it hugely disfavors readability.

I don't understand the handle validation, it's true that the UTF-8 representation of "." is 2e which is less than the representation of "0" which is 30, however, the check is there because if the character is equal to 2e then we should not revert, so as far as I can tell this is a necessary check.

We only accept the period, the range between "0" and "9" as well as the range between "a" and "b."

The final gas optimization is valid, good catch!

Zer0dot commented 2 years ago

Correction, because it's assumed most users will use a follow module, this optimization (which does indeed reduce costs when there is no follow module to set) appears to increase costs in scenarios where users have follow modules.

Zer0dot commented 2 years ago

Unchecked increments are valid though!

Zer0dot commented 2 years ago

Specifically not addressed in a PR but in a commit: https://github.com/aave/lens-protocol/commit/589155643c98881d084dc32f8a55537d645a1f9b