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

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

Gas Optimizations #205

code423n4 opened this issue Jul 19, 2022 · 0 comments
Labels
bug Something isn't working G (Gas Optimization)

Comments

@code423n4
Copy link
Contributor

Summary

Issue Instances
++i uses less gas compared to i++ 5
Let the default value be applied to variables initialized to the default value 16
Use custom errors instead of require() to save gas 26
Cache array length outside of loop 3
Return values directly without an intermediate return variable 2
Use named return variable 13
Named return variable is not used 4
Named return variable in function declaration makes return() redundant 3
Use != 0 instead of > 0 for a uint 3

Gas Optimisations

++i uses less gas compared to i++

This is especially relevant for the use of i++ in for loops. This saves 6 gas per loop.

See: https://github.com/byterocket/c4-common-issues/blob/main/0-Gas-Optimizations.md/#g012---use-prefix-increment-instead-of-postfix-increment-if-possible

There are 5 instances of this issue:

FILE: contracts/dnssec-oracle/BytesUtils.sol

266:     for(uint i = 0; i < len; i++) {

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/dnssec-oracle/BytesUtils.sol#L266

FILE: contracts/dnssec-oracle/BytesUtils.sol

313:     for(uint256 idx = off; idx < off + len; idx++) {

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/dnssec-oracle/BytesUtils.sol#L313

FILE: contracts/dnssec-oracle/DNSSECImpl.sol

93:       for(uint i = 0; i < input.length; i++) {

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/dnssec-oracle/DNSSECImpl.sol#L93

FILE: contracts/ethregistrar/ETHRegistrarController.sol

256:       for (uint256 i = 0; i < data.length; i++) {

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/ethregistrar/ETHRegistrarController.sol#L256

FILE: contracts/ethregistrar/StringUtils.sol

14:        for(len = 0; i < bytelength; len++) {

https://github.com/code-423n4/2022-07-ens/blob/ff6e59b9415d0ead7daf31c2ed06e86d9061ae22/contracts/ethregistrar/StringUtils.sol#L14

Let the default value be applied to variables initialized to the default value

Letting the default value of 0, false be initialized to variables costs less gas compared to initializing it to these default values.

See: https://github.com/byterocket/c4-common-issues/blob/main/0-Gas-Optimizations.md/#g001---dont-initialize-variables-with-default-value

There are 16 instances of this issue:

FILE: contracts/dnssec-oracle/BytesUtils.sol

56:      for (uint idx = 0; idx < shortest; idx += 32) {

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/dnssec-oracle/BytesUtils.sol#L56

FILE: contracts/dnssec-oracle/BytesUtils.sol

264:     uint ret = 0;

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/dnssec-oracle/BytesUtils.sol#L264

FILE: contracts/dnssec-oracle/BytesUtils.sol

266:     for(uint i = 0; i < len; i++) {

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/dnssec-oracle/BytesUtils.sol#L266

FILE: contracts/dnssec-oracle/DNSSECImpl.sol

93:      for(uint i = 0; i < input.length; i++) {

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/dnssec-oracle/DNSSECImpl.sol#L93

FILE: contracts/dnssec-oracle/RRUtils.sol

50:      uint count = 0;

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/dnssec-oracle/RRUtils.sol#L50

FILE: contracts/dnssec-oracle/RRUtils.sol

63:      uint constant RRSIG_TYPE = 0;

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/dnssec-oracle/RRUtils.sol#L63

FILE: contracts/dnssec-oracle/RRUtils.sol

181:     uint constant DNSKEY_FLAGS = 0; 

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/dnssec-oracle/RRUtils.sol#L181

FILE: contracts/dnssec-oracle/RRUtils.sol

200:     uint constant DS_KEY_TAG = 0;

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/dnssec-oracle/RRUtils.sol#L200

FILE: contracts/dnssec-oracle/RRUtils.sol

310:     for(uint i = 0; i < data.length + 31; i += 32) {

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/dnssec-oracle/RRUtils.sol#L310

FILE: contracts/ethregistrar/ETHRegistrar.sol

256:     for (uint256 i = 0; i < data.length; i++) {

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/ethregistrar/ETHRegistrarController.sol#L256

FILE: contracts/ethregistrar/StringUtils.sol

12:      uint i = 0;

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/ethregistrar/StringUtils.sol#L12

FILE: contracts/ethregistrar/StringUtils.sol

14:      for(len = 0; i < bytelength; len++) {

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/ethregistrar/StringUtils.sol#L14

FILE: contracts/wrapper/ERC1155Fuse.sol

92:      for (uint256 i = 0; i < accounts.length; ++i) {

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/wrapper/ERC1155Fuse.sol#L92

FILE: contracts/wrapper/ERC1155Fuse.sol

145:     fuses = 0;

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/wrapper/ERC1155Fuse.sol#L145

FILE: contracts/wrapper/ERC1155Fuse.sol

205:     for (uint256 i = 0; i < ids.length; ++i) {

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/wrapper/ERC1155Fuse.sol#L205

FILE: contracts/wrapper/NameWrapper.sol

16:      uint32 constant CAN_DO_EVERYTHING = 0;

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/wrapper/INameWrapper.sol#L16

Use custom errors instead of require() to save gas

Custom errors are available from solidity version 0.8.4. The instances below match or exceed that version.

There are 26 instances of this issue:

FILE: contracts/dnssec-oracle/RRUtils.sol

307:     require(data.length <= 8192, "Long keys not permitted");

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/dnssec-oracle/RRUtils.sol#L307

FILE: contracts/ethregistrar/ETHRegistrarController.sol

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

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/ethregistrar/ETHRegistrarController.sol#L99-L102

FILE: contracts/ethregistrar/ETHRegistrarController.sol

137     require(
138         msg.value >= (price.base + price.premium),
139         "ETHRegistrarController: Not enough ether provided"
140     );

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/ethregistrar/ETHRegistrarController.sol#L137-L140

FILE: contracts/ethregistrar/ETHRegistrarController.sol

196     require(
197         msg.value >= price.base,
198         "ETHController: Not enough Ether provided for renewal"
199     );

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/ethregistrar/ETHRegistrarController.sol#L196-L199

FILE: contracts/ethregistrar/ETHRegistrarController.sol

232     require(
233         commitments[commitment] + minCommitmentAge <= block.timestamp,
234         "ETHRegistrarController: Commitment is not valid"
235     );

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/ethregistrar/ETHRegistrarController.sol#L232-L235

FILE: contracts/ethregistrar/ETHRegistrarController.sol

238     require(
239         commitments[commitment] + maxCommitmentAge > block.timestamp,
240         "ETHRegistrarController: Commitment has expired"
241     );

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/ethregistrar/ETHRegistrarController.sol#L238-L241

FILE: contracts/ethregistrar/ETHRegistrarController.sol

242     require(available(name), "ETHRegistrarController: Name is unavailable");

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/ethregistrar/ETHRegistrarController.sol#L242

FILE: contracts/ethregistrar/ETHRegistrarController.sol

259     require(
260         txNamehash == nodehash,
261         "ETHRegistrarController: Namehash on record do not match the name being registered"
262     );

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/ethregistrar/ETHRegistrarController.sol#L259-L262

FILE: contracts/registry/ReverseRegistrar.sol

41      require(
42          addr == msg.sender ||
43              controllers[msg.sender] ||
44              ens.isApprovedForAll(addr, msg.sender) ||
45              ownsContract(addr),
46          "ReverseRegistrar: Caller is not a controller or authorised by address or the address itself"
47     );

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/registry/ReverseRegistrar.sol#L41-L47

FILE: contracts/registry/ReverseRegistrar.sol

52     require(
53         address(resolver) != address(0),
54         "ReverseRegistrar: Resolver address must not be 0"
55     );

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/registry/ReverseRegistrar.sol#L52-L55

FILE: contracts/wrapper/BytesUtil.sol

28     require(offset == self.length - 1, "namehash: Junk at end of name");

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/wrapper/BytesUtil.sol#L28

FILE: contracts/wrapper/BytesUtil.sol

42     require(idx < self.length, "readLabel: Index out of bounds");

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/wrapper/BytesUtil.sol#L42

FILE: contracts/wrapper/ERC1155Fuse.sol

60     require(
61         account != address(0),
62         "ERC1155: balance query for the zero address"
63     );

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/wrapper/ERC1155Fuse.sol#L60-L63

FILE: contracts/wrapper/ERC1155Fuse.sol

85      require(
86          accounts.length == ids.length,
87          "ERC1155: accounts and ids length mismatch"
88      );

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/wrapper/ERC1155Fuse.sol#L85-L88

FILE: contracts/wrapper/ERC1155Fuse.sol

107     require(
108         msg.sender != operator,
109         "ERC1155: setting approval status for self"
110     );

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/wrapper/ERC1155Fuse.sol#L107-L110

FILE: contracts/wrapper/ERC1155Fuse.sol

176    require(to != address(0), "ERC1155: transfer to the zero address");

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/wrapper/ERC1155Fuse.sol#L176

FILE: contracts/wrapper/ERC1155Fuse.sol

177     require(
178         from == msg.sender || isApprovedForAll(from, msg.sender),
179         "ERC1155: caller is not owner nor approved"
180     );

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/wrapper/ERC1155Fuse.sol#L177-L180

FILE: contracts/wrapper/ERC1155Fuse.sol

195     require(
196         ids.length == amounts.length,
197         "ERC1155: ids and amounts length mismatch"
198     );

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/wrapper/ERC1155Fuse.sol#L195-L198

FILE: contracts/wrapper/ERC1155Fuse.sol

199     require(to != address(0), "ERC1155: transfer to the zero address");

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/wrapper/ERC1155Fuse.sol#L199

FILE: contracts/wrapper/ERC1155Fuse.sol

200     require(
201         from == msg.sender || isApprovedForAll(from, msg.sender),
202         "ERC1155: transfer caller is not owner nor approved"
203     );

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/wrapper/ERC1155Fuse.sol#L200-L203

FILE: contracts/wrapper/ERC1155Fuse.sol

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

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/wrapper/ERC1155Fuse.sol#L215-L218

FILE: contracts/wrapper/ERC1155Fuse.sol

248     require(owner == address(0), "ERC1155: mint of existing token");

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/wrapper/ERC1155Fuse.sol#L248

FILE: contracts/wrapper/ERC1155Fuse.sol

249     require(newOwner != address(0), "ERC1155: mint to the zero address");

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/wrapper/ERC1155Fuse.sol#L249

FILE: contracts/wrapper/ERC1155Fuse.sol

250     require(
251         newOwner != address(this),
252         "ERC1155: newOwner cannot be the NameWrapper contract"
253     );

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/wrapper/ERC1155Fuse.sol#L250-L253

FILE: contracts/wrapper/ERC1155Fuse.sol

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

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/wrapper/ERC1155Fuse.sol#L290-L293

FILE: contracts/wrapper/Controllable.sol

17      require(controllers[msg.sender], "Controllable: Caller is not a controller");

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/wrapper/Controllable.sol#L17

Cache array length outside of loop

Reading array length at each iteration of the loop takes 6 gas (3 for mload and 3 to place memory_offset) in the stack.

Caching the array length in the stack saves around 3 gas per iteration. To do this, create a variables containing the array length before the loop.

See: https://github.com/byterocket/c4-common-issues/blob/main/0-Gas-Optimizations.md/#g002---cache-array-length-outside-of-loop

There are 3 instances of this issue:

FILE: contracts/ethregistrar/ETHRegistrarController.sol

256     for (uint256 i = 0; i < data.length; i++) {

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/ethregistrar/ETHRegistrarController.sol#L256

FILE: contracts/wrapper/ERC1155Fuse.sol

92      for (uint256 i = 0; i < accounts.length; ++i) {

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/wrapper/ERC1155Fuse.sol#L92

FILE: contracts/wrapper/ERC1155Fuse.sol

205     for (uint256 i = 0; i < ids.length; ++i) {

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/wrapper/ERC1155Fuse.sol#L205

Return values directly without an intermediate return variable

Initializing a return variable for a function, then assigning a value to it requires more gas compared to simply returning the value, as long as the variable is not being used elsewhere in the function.

There are 2 instances of this issue:

FILE: contracts/ethregistrar/ETHRegistrarController.sol

67    function rentPrice(string memory name, uint256 duration)

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/ethregistrar/ETHRegistrarController.sol#L67

FILE: /contracts/wrapper/NameWrapper.sol

251    function registerAndWrapETH2LD(

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/wrapper/NameWrapper.sol#L251

Use named return variable

Using a named return variable in the function declaration saves gas.

There are 13 instances of this issue:

FILE: contracts/dnssec-oracle/BytesUtils.sol#L234

234    function substring(bytes memory self, uint offset, uint len) internal pure returns(bytes memory) {

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/dnssec-oracle/BytesUtils.sol#L234

FILE: contracts/dnssec-oracle/DNSSECImpl.sol

91    function verifyRRSet(RRSetWithSignature[] memory input, uint256 now) public virtual view override returns(bytes memory) {

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/dnssec-oracle/DNSSECImpl.sol#L91

FILE: contracts/dnssec-oracle/RRUtils.sol

49    function labelCount(bytes memory self, uint offset) internal pure returns(uint) {

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/dnssec-oracle/RRUtils.sol#L49

FILE: contracts/ethregistrar/StringUtils.sol

10    function strlen(string memory s) internal pure returns (uint) {

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/ethregistrar/StringUtils.sol#L10

FILE: contracts/registry/ReverseRegistrar.sol

132   function setNameForAddr(

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/registry/ReverseRegistrar.sol#L132

FILE: contracts/wrapper/ERC1155Fuse.sol#L25

25    function ownerOf(uint256 id) public view virtual returns (address) {

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/wrapper/ERC1155Fuse.sol#L25

FILE: contracts/wrapper/ERC1155Fuse.sol

78    function balanceOfBatch(address[] memory accounts, uint256[] memory ids)

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/wrapper/ERC1155Fuse.sol#L78

FILE: contracts/wrapper/NameWrapper.sol

373    function setFuses(bytes32 node, uint32 fuses)

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/wrapper/NameWrapper.sol#L373

FILE: contracts/wrapper/NameWrapper.sol

373    function setFuses(bytes32 node, uint32 fuses)

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/wrapper/NameWrapper.sol#L373

FILE: contracts/wrapper/NameWrapper.sol

825    function _getDataAndNormaliseExpiry(

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/wrapper/NameWrapper.sol#L825

FILE: contracts/wrapper/NameWrapper.sol

846    function _getETH2LDDataAndNormaliseExpiry(

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/wrapper/NameWrapper.sol#L846

FILE: contracts/wrapper/NameWrapper.sol

867    function _normaliseExpiry(

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/wrapper/NameWrapper.sol#L867

FILE: contracts/wrapper/NameWrapper.sol

885    function _wrapETH2LD(

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/wrapper/NameWrapper.sol#L885

Named return variable is not used

Remove the named return variable to save 25 gas per function call.

There are 4 instances of this issue:

FILE: contracts/dnssec-oracle/BytesUtils.sol

135   function readUint8(bytes memory self, uint idx) internal pure returns (uint8 ret) {

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/dnssec-oracle/BytesUtils.sol#L135

FILE: contracts/dnssec-oracle/RRUtils.sol

38    function readName(bytes memory self, uint offset) internal pure returns(bytes memory ret) {

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/dnssec-oracle/RRUtils.sol#L38

FILE: contracts/wrapper/NameWrapper.sol

90    function ownerOf(uint256 id)

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/wrapper/NameWrapper.sol#L90

FILE: contracts/wrapper/NameWrapper.sol

741    function _addLabel(string memory label, bytes memory name)

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/wrapper/NameWrapper.sol#L741

Named return variable in function declaration makes return() redundant

Since there is a named return variable specified in the function declaration, the return function is redundant. The function will revert the named return variable upon completion of the function.

There are 3 instances of this issue:

FILE: contracts/dnssec-oracle/DNSSECImpl.sol

110    function validateSignedSet(RRSetWithSignature memory input, bytes memory proof, uint256 now) internal view returns(RRUtils.SignedSet memory rrset) {

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/dnssec-oracle/DNSSECImpl.sol#L110

FILE: contracts/wrapper/NameWrapper.sol

825    function _getDataAndNormaliseExpiry(

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/wrapper/NameWrapper.sol#L825

FILE: contracts/wrapper/NameWrapper.sol

846    function _getETH2LDDataAndNormaliseExpiry(

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/wrapper/NameWrapper.sol#L846

Use != 0 instead of > 0 for a uint

Since the integers are unsigned, != 0 and > 0 are equivalent. Using != 0 is 6 gas per instance cheaper than > 0

See: https://github.com/byterocket/c4-common-issues/blob/main/0-Gas-Optimizations.md/#g003---use--0-instead-of--0-for-unsigned-integer-comparison

There are 3 instances of this issue:

FILE: contracts/dnssec-oracle/RRUtils.sol

245    while (counts > 0 && !self.equals(off, other, otheroff)) {

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/dnssec-oracle/RRUtils.sol#L245

FILE: contracts/ethregistrar/ETHRegistrarController.sol

98     if (data.length > 0) {

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/ethregistrar/ETHRegistrarController.sol#L98

FILE: contracts/wrapper/BytesUtil.sol

44     if(len > 0) {

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/wrapper/BytesUtil.sol#L44

@code423n4 code423n4 added bug Something isn't working G (Gas Optimization) labels Jul 19, 2022
code423n4 added a commit that referenced this issue Jul 19, 2022
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

1 participant