Gas Optimizations

[code423n4]

NO NEED TO EXPLICITLY INITIALIZE VARIABLES WITH DEFAULT VALUES

If a variable is not set/initialized, it is assumed to have the default value (0 for uint, false for bool, address(0) for address…). Explicitly initializing it with its default value is an anti-pattern and wastes gas.

src/governance/treasury/Treasury.sol
162:        for (uint256 i = 0; i < numTargets; ++i) {

src/token/metadata/MetadataRenderer.sol
119:        for (uint256 i = 0; i < numNewProperties; ++i) {
133:        for (uint256 i = 0; i < numNewItems; ++i) {
189:        for (uint256 i = 0; i < numProperties; ++i) {
229:        for (uint256 i = 0; i < numProperties; ++i) {

The demo of the loop gas comparison can be seen here.

X = X + Y / X = X - Y IS CHEAPER THAN X += Y / X -= Y

The demo of the gas comparison can be seen here.

Consider use X = X + Y / X = X - Y to save gas.

src/governance/governor/Governor.sol
280:        proposal.againstVotes += uint32(weight);
285:        proposal.forVotes += uint32(weight);
290:        proposal.abstainVotes += uint32(weight);

src/token/Token.sol
88:         if ((totalOwnership += uint8(founderPct)) > 100) revert INVALID_FOUNDER_OWNERSHIP();
118:        (baseTokenId += schedule) % 100;

src/token/metadata/MetadataRenderer.sol
140:        _propertyId += numStoredProperties;

src/token/metadata/MetadataRenderer.sol
197:        seed >>= 16;

EXPRESSIONS FOR CONSTANT VALUES SUCH AS A CALL TO keccak256(),` SHOULD USE IMMUTABLE RATHER THAN CONSTANT

Constant expressions are left as expressions, not constants.

When a constant declared as:

src/lib/token/ERC721Votes.sol
21:     bytes32 internal constant DELEGATION_TYPEHASH = keccak256("Delegation(address from,address to,uint256 nonce,uint256 deadline)");

src/lib/utils/EIP712.sol
19:     bytes32 internal constant DOMAIN_TYPEHASH = keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)");

It is expected that the value should be converted into a constant value at compile time. But the expression is re-calculated each time the constant is referenced.

consequences:

• each usage of a "constant" costs ~100gas more on each access (it is still a little better than storing the result in storage, but not much..)
• since these are not real constants, they can't be referenced from a real constant environment (e.g. from assembly, or from another library )

Suggestion:
Change these expressions from constant to immutable and implement the calculation in the constructor or hardcode these values in the constants and add a comment to say how the value was calculated.

reference:
ethereum/solidity#9232

Use bytes32 instead of string

Use bytes32 instead of string to save gas whenever possible.
String is a dynamic data structure and therefore is more gas consuming then bytes32.

Bytes32 can be used instead of string in the following:

src/token/metadata/types/MetadataRendererTypesV1.sol
    struct ItemParam {
        uint256 propertyId;
        string name;
        bool isNewProperty;
    }

    struct IPFSGroup {
        string baseUri;
        string extension;
    }

    struct Item {
        uint16 referenceSlot;
        string name;
    }

    struct Property {
        string name;
        Item[] items;
    }

    struct Settings {
        address token;
        address treasury;
        string name;
        string description;
        string contractImage;
        string rendererBase;
    }

USE CALLDATA INSTEAD OF MEMORY

When arguments are read-only on external functions, the data location can be calldata.

The demo of the gas comparison can be seen here.

src/governance/governor/Governor.sol
116-121:
    function propose(
        address[] memory _targets,
        uint256[] memory _values,
        bytes[] memory _calldatas,
        string memory _description
    ) external returns (bytes32) {

192-196:
    function castVoteWithReason(
        bytes32 _proposalId,
        uint256 _support,
        string memory _reason
    ) external returns (uint256) {

FUNCTIONS ONLY CALLED OUTSIDE OF THE CONTRACT CAN BE DECLARED AS EXTERNAL

External call cost is less expensive than of public functions.

The difference is because in public functions, Solidity immediately copies array arguments to memory, while external functions can read directly from calldata. Memory allocation is expensive, whereas reading from calldata is cheap.

src/token/metadata/MetadataRenderer.sol
206:    function getAttributes(uint256 _tokenId) public view returns (bytes memory aryAttributes, bytes memory queryString) {

MULTIPLE ADDRESS MAPPINGS CAN BE COMBINED INTO A SINGLE MAPPING OF AN ADDRESS TO A STRUCT WHERE APPROPRIATE

Saves a storage slot for the mapping. Depending on the circumstances and sizes of types, can avoid a Gsset (20000 gas) per mapping combined. Reads and subsequent writes can also be cheaper when a function requires both values and they both fit in the same storage slot.

src/governance/governor/storage/GovernorStorageV1.sol
14-19:
    /// @dev Proposal Id => Proposal
    mapping(bytes32 => Proposal) internal proposals;

    /// @dev Proposal Id => User => Has Voted
    mapping(bytes32 => mapping(address => bool)) internal hasVoted;

src/lib/token/ERC721.sol
24-34:
    /// @dev ERC-721 token id => Owner
    mapping(uint256 => address) internal owners;

    /// @dev ERC-721 token id => Manager
    mapping(uint256 => address) internal tokenApprovals;

28-38:
    /// @dev Owner => Balance
    mapping(address => uint256) internal balances;

    /// @dev Owner => Operator => Approved
    mapping(address => mapping(address => bool)) internal operatorApprovals;

src/lib/token/ERC721Votes.sol
28-37:
    /// @notice Account => Delegate
    mapping(address => address) internal delegation;

    /// @dev Account => Num Checkpoints
    mapping(address => uint256) internal numCheckpoints;

    /// @dev Account => Checkpoint Id => Checkpoint
    mapping(address => mapping(uint256 => Checkpoint)) internal checkpoints;

Use bit shift instead of power operation or * 2, / 2

`` seem to be frequently called functions.

src/lib/token/ERC721Votes.sol

        middle = high - (high - low) / 2;

Suggestion:
Change the above to

        middle = high - (high - low) >> 1;

The demo of the gas comparison can be seen here.

[GalloDaSballo]

About 500 gas, mostly from the calldata / memory

[GalloDaSballo]

500