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 #174

Open
code423n4 opened this issue Aug 6, 2022 · 0 comments
Open

Gas Optimizations #174

code423n4 opened this issue Aug 6, 2022 · 0 comments
Labels

Comments

@code423n4
Copy link
Contributor

Gas Optimizations

Finding Instances
[G-01] Multiple address mappings can be combined into a single mapping of an address to a struct, where appropriate 1
[G-02] require()/revert() checks used multiples times should be turned into a function or modifier 4
[G-03] bool is gas inefficient when used in storage 3
[G-04] Redundant require check 1
[G-05] Implementing return is redundant if function already has a named returns method implemented 2
[G-06] array.length should be cached in for loop 2
[G-07] for loop increments should be unchecked{} if overflow is not possible 6
[G-08] Using prefix(++i) instead of postfix (i++) saves gas 7
[G-09] Setting variable to default value is redundant 4
[G-10] Use custom errors rather than revert()/require() strings to save gas 20

Gas overview per contract

Contract Instances Gas Ops
Project.sol 29 7
HomeFi.sol 11 5
DebtToken.sol 6 2
Community.sol 4 4

Gas Optimizations

[G-01] 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. Finally, if both fields are accessed in the same function, can save ~42 gas per access due to not having to recalculate the key's keccak256 hash (Gkeccak256 - 30 gas) and that calculation's associated stack operatio
1 instance of this issue has been found:

[G-01] HomeFi.sol#L64-L66
    mapping(address => uint256) public override projectTokenId;
    /// @inheritdoc IHomeFi
    mapping(address => address) public override wrappedToken;

[G-02] require()/revert() checks used multiples times should be turned into a function or modifier

Doing so increases code readability decreases number of instructions for the compiler.
4 instances of this issue have been found:

[G-02] Project.sol#L153-L154
        require(contractor != address(0), "Project::0 address");
[G-02b] Project.sol#L135-L136
        require(_contractor != address(0), "Project::0 address");
[G-02c] DebtToken.sol#L96-L97
        revert("DebtToken::blocked");
[G-02d] DebtToken.sol#L104-L105
        revert("DebtToken::blocked");

[G-03] bool is gas inefficient when used in storage

Instead use uint256 values to represent true/false instead.
Reference
3 instances of this issue have been found:

[G-03] Project.sol#L78-L79
    bool public override contractorDelegated;
[G-03b] Project.sol#L68-L69
    bool public override contractorConfirmed;
[G-03c] HomeFi.sol#L50-L51
    bool public override addrSet;

[G-04] Redundant require check

Modifier noChange can be used instead.
1 instance of this issue has been found:

[G-04] HomeFi.sol#L191-L192
        require(lenderFee != _newLenderFee, "HomeFi::!Change");

[G-05] Implementing return is redundant if function already has a named returns method implemented

Redundant return methods increase gas on deployment and execution.
2 instances of this issue have been found:

[G-05] HomeFi.sol#L318-L321
    returns (bytes calldata)
    {
        // We want to use the _msgData() implementation of ERC2771ContextUpgradeable
        return super._msgData();
[G-05b] HomeFi.sol#L307-L310
        returns (address sender)
    {
        // We want to use the _msgSender() implementation of ERC2771ContextUpgradeable
        return super._msgSender();

[G-06] array.length should be cached in for loop

Caching the length array would save gas on each iteration by not having to read from memory or storage multiple times.
Example:

uint length = array.length;
for (uint i; i < length;){
		uncheck { ++i }
}

2 instances of this issue have been found:

[G-06] Community.sol#L624-L625
        for (uint256 i = 0; i < _communities[_communityID].memberCount; i++) {
[G-06b] Project.sol#L603-L604
            for (; i < _changeOrderedTask.length; i++) {

[G-07] for loop increments should be unchecked{} if overflow is not possible

From Solidity 0.8.0 onwards using the unchecked keyword saves 30 to 40 gas per loop.
Example:

uint length = array.length;
for (uint i; i < length;){
		uncheck { ++i }
}

6 instances of this issue have been found:

[G-07] Community.sol#L624-L625
        for (uint256 i = 0; i < _communities[_communityID].memberCount; i++) {
[G-07b] Project.sol#L650-L651
            for (++j; j <= taskCount; j++) {
[G-07c] Project.sol#L322-L323
        for (uint256 i = 0; i < _length; i++) {
[G-07d] Project.sol#L311-L312
        for (uint256 i = 0; i < _length; i++) {
[G-07e] Project.sol#L248-L249
        for (uint256 i = 0; i < _length; i++) {
[G-07f] Project.sol#L603-L604
            for (; i < _changeOrderedTask.length; i++) {

[G-08] Using prefix(++i) instead of postfix (i++) saves gas

It saves 6 gas per iteration.
7 instances of this issue have been found:

[G-08] Community.sol#L624-L625
        for (uint256 i = 0; i < _communities[_communityID].memberCount; i++) {
[G-08b] Project.sol#L650-L651
            for (++j; j <= taskCount; j++) {
[G-08c] Project.sol#L322-L323
        for (uint256 i = 0; i < _length; i++) {
[G-08d] Project.sol#L311-L312
        for (uint256 i = 0; i < _length; i++) {
[G-08e] Project.sol#L248-L249
        for (uint256 i = 0; i < _length; i++) {
[G-08f] Project.sol#L625-L626
                    _loopCount++;
[G-08g] Project.sol#L603-L604
            for (; i < _changeOrderedTask.length; i++) {

[G-09] Setting variable to default value is redundant

Setting variable to default value is redundant.
4 instances of this issue have been found:

[G-09] Community.sol#L624-L625
        for (uint256 i = 0; i < _communities[_communityID].memberCount; i++) {
[G-09b] Project.sol#L322-L323
        for (uint256 i = 0; i < _length; i++) {
[G-09c] Project.sol#L311-L312
        for (uint256 i = 0; i < _length; i++) {
[G-09d] Project.sol#L248-L249
        for (uint256 i = 0; i < _length; i++) {

[G-10] Use custom errors rather than revert()/require() strings to save gas

Custom errors save ~50 gas each time they're hit by avoiding having to allocate and store the revert string.
20 instances of this issue have been found:

[G-10] Project.sol#L341-L342
        require(_projectAddress == address(this), "Project::!Project");
[G-10b] Project.sol#L199-L202
        require(
            projectCost() >= uint256(_newTotalLent),
            "Project::value>required"
        );
[G-10c] Project.sol#L195-L196
        require(_cost > 0, "Project::!value>0");
[G-10d] Project.sol#L189-L192
        require(
            _sender == builder || _sender == homeFi.communityContract(),
            "Project::!Builder&&!Community"
        );
[G-10e] Project.sol#L176-L177
        require(_nonce == hashChangeNonce, "Project::!Nonce");
[G-10f] Project.sol#L153-L154
        require(contractor != address(0), "Project::0 address");
[G-10g] Project.sol#L150-L151
        require(_msgSender() == builder, "Project::!B");
[G-10h] Project.sol#L135-L136
        require(_contractor != address(0), "Project::0 address");
[G-10i] Project.sol#L132-L133
        require(_projectAddress == address(this), "Project::!projectAddress");
[G-10j] Project.sol#L123-L124
        require(!contractorConfirmed, "Project::GC accepted");
[G-10k] HomeFi.sol#L255-L260
        require(
            _currency == tokenCurrency1 ||
                _currency == tokenCurrency2 ||
                _currency == tokenCurrency3,
            "HomeFi::!Currency"
        );
[G-10l] HomeFi.sol#L191-L192
        require(lenderFee != _newLenderFee, "HomeFi::!Change");
[G-10m] HomeFi.sol#L142-L143
        require(!addrSet, "HomeFi::Set");
[G-10n] HomeFi.sol#L84-L85
        require(_oldAddress != _newAddress, "HomeFi::!Change");
[G-10o] HomeFi.sol#L78-L79
        require(_address != address(0), "HomeFi::0 address");
[G-10p] HomeFi.sol#L73-L74
        require(admin == _msgSender(), "HomeFi::!Admin");
[G-10q] DebtToken.sol#L31-L35
        require(
            communityContract == _msgSender(),
            "DebtToken::!CommunityContract"
        );
        _;
[G-10r] DebtToken.sol#L50-L51
        require(_communityContract != address(0), "DebtToken::0 address");
[G-10s] DebtToken.sol#L96-L97
        revert("DebtToken::blocked");
[G-10t] DebtToken.sol#L104-L105
        revert("DebtToken::blocked");
@code423n4 code423n4 added bug Something isn't working G (Gas Optimization) labels Aug 6, 2022
code423n4 added a commit that referenced this issue Aug 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants