Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Gas Optimizations #79

Open
code423n4 opened this issue Feb 16, 2022 · 4 comments
Open

Gas Optimizations #79

code423n4 opened this issue Feb 16, 2022 · 4 comments
Labels
bug Something isn't working G (Gas Optimization)

Comments

@code423n4
Copy link
Contributor

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);
    }
@code423n4 code423n4 added bug Something isn't working G (Gas Optimization) labels Feb 16, 2022
code423n4 added a commit that referenced this issue Feb 16, 2022
@Zer0dot
Copy link
Collaborator

Zer0dot commented Mar 21, 2022

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
Copy link
Collaborator

Zer0dot commented Mar 21, 2022

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
Copy link
Collaborator

Zer0dot commented Mar 24, 2022

Unchecked increments are valid though!

@Zer0dot
Copy link
Collaborator

Zer0dot commented Mar 24, 2022

Specifically not addressed in a PR but in a commit: lens-protocol/core@5891556

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working G (Gas Optimization)
Projects
None yet
Development

No branches or pull requests

2 participants