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

Open
code423n4 opened this issue Jul 19, 2022 · 0 comments
Open

Gas Optimizations #107

code423n4 opened this issue Jul 19, 2022 · 0 comments

Comments

@code423n4
Copy link
Contributor

Require revert string is too long

The 22 require revert strings referenced below should be shortened to 32 characters or fewer to save gas or else consider using custom error codes (which could save even more gas)

Example:

ETHRegistrarController.sol: L99-102

            require(
                resolver != address(0),
                "ETHRegistrarController: resolver is required when data is supplied"
            );

Additional long require strings:

ETHRegistrarController.sol: L137-140

ETHRegistrarController.sol: L196-199

ETHRegistrarController.sol: L232-235

ETHRegistrarController.sol: L238-241

ETHRegistrarController.sol: L242

ETHRegistrarController.sol: L259-262

ReverseRegistrar.sol: L41-47

ReverseRegistrar.sol: L52-55

ERC1155Fuse.sol: L60-63

ERC1155Fuse.sol: L85-88

ERC1155Fuse.sol: L107-110

ERC1155Fuse.sol: L176

ERC1155Fuse.sol: L177-180

ERC1155Fuse.sol: L195-198

ERC1155Fuse.sol: L199

ERC1155Fuse.sol: L200-203

ERC1155Fuse.sol: L215-218

ERC1155Fuse.sol: L249

ERC1155Fuse.sol: L250-253

ERC1155Fuse.sol: L290-293

Controllable.sol.sol: L17



Revert error string is too long

The revert strings below should be shortened to 32 characters or fewer to save gas, or else consider using custom errors


ERC1155Fuse.sol: L322-327

                    revert("ERC1155: ERC1155Receiver rejected tokens");
                }
            } catch Error(string memory reason) {
                revert(reason);
            } catch {
                revert("ERC1155: transfer to non ERC1155Receiver implementer");

ERC1155Fuse.sol: L354-359

                    revert("ERC1155: ERC1155Receiver rejected tokens");
                }
            } catch Error(string memory reason) {
                revert(reason);
            } catch {
                revert("ERC1155: transfer to non ERC1155Receiver implementer");


Use of '&&' within a require function

Splitting such functions into separate require() statements (instead of using &&) saves gas


BytesUtils.sol: L268

            require(char >= 0x30 && char <= 0x7A);

Recommendation:

            require(char >= 0x30, "Error message");
            require(char <= 0x7A, "Error message");

The same require function with embedded && occurs in both sets of lines referenced below:

ERC1155Fuse.sol: L215-218

ERC1155Fuse.sol: L290-293

            require(
                amount == 1 && oldOwner == from,
                "ERC1155: insufficient balance for transfer"
            );

Recommendation:

            require(amount == 1, "Error message");
            require(oldOwner == from, "Error message");


Array length should not be looked up in every iteration of a for loop

Since calculating the array length costs gas, it's best to read the length of the array from memory before executing the loop


ERC1155Fuse.sol: L92-94

        for (uint256 i = 0; i < accounts.length; ++i) {
            batchBalances[i] = balanceOf(accounts[i], ids[i]);
        }

Suggestion:

        uint256 totalAccountsLength = accounts.length; 
        for (uint256 i = 0; i < totalAccountsLength; ++i) {
            batchBalances[i] = balanceOf(accounts[i], ids[i]);
        }

Similarly for the following for loops:

ERC1155Fuse.sol: L205-213

ETHRegistrarController.sol: L256-267



Use ++i instead of i++ to increase count in a for loop

Since use of i++ costs more gas, it is better to use ++i in the for loop below:

ETHRegistrarController.sol: L256-267



Avoid use of default "checked" behavior in a for loop

Underflow/overflow checks are made every time ++i (or i++) is called. Such checks are unnecessary since i is already limited. Therefore, use unchecked{++i}/unchecked{i++} instead


ERC1155Fuse.sol: L92-94

        for (uint256 i = 0; i < accounts.length; ++i) {
            batchBalances[i] = balanceOf(accounts[i], ids[i]);
        }

Suggestion:

        uint256 totalAccountsLength = accounts.length; 
        for (uint256 i = 0; i < totalAccountsLength;) {
            batchBalances[i] = balanceOf(accounts[i], ids[i]);

            unchecked{
              ++i;
          }
        }

Similarly for the following for loops:

ERC1155Fuse.sol: L205-213

ETHRegistrarController.sol: L256-267



Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant