Gas Optimizations #249

code423n4 · 2022-07-19T17:58:50Z

1. An array’s length should be cached to save gas in for-loops

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.

Instances:

contracts/ethregistrar/ETHRegistrarController.sol:256:
contracts/dnssec-oracle/DNSSECImpl.sol:93:
contracts/dnssec-oracle/RRUtils.sol:310:
contracts/wrapper/ERC1155Fuse.sol:205:
contracts/wrapper/ERC1155Fuse.sol:92:

contracts/ethregistrar/ETHRegistrarController.sol:256:        for (uint256 i = 0; i < data.length; i++) {
contracts/dnssec-oracle/DNSSECImpl.sol:93:        for(uint i = 0; i < input.length; i++) {
contracts/dnssec-oracle/RRUtils.sol:310:            for(uint i = 0; i < data.length + 31; i += 32) {
contracts/wrapper/ERC1155Fuse.sol:92:        for (uint256 i = 0; i < accounts.length; ++i) {
contracts/wrapper/ERC1155Fuse.sol:205:        for (uint256 i = 0; i < ids.length; ++i) {

Remediation:

Here, I suggest storing the array’s length in a variable before the for-loop, and use it instead.

2. Require instead of &&

Require statements including conditions with the && operator can be broken down in multiple require statements to save gas.

Instances include:

contracts/wrapper/ERC1155Fuse.sol::215
contracts/wrapper/ERC1155Fuse.sol:290
contracts/dnssec-oracle/BytesUtils.sol:268

contracts/wrapper/ERC1155Fuse.sol:
  215:        require( amount == 1 && oldOwner == from,
  290:        require(amount == 1 && oldOwner == from,
contracts/dnssec-oracle/BytesUtils.sol:268:        require(char >= 0x30 && char <= 0x7A);

Mitigation:

Breakdown each condition in a separate require statement (though require statements should be replaced with custom errors)

3. ++i costs less gas than i++, especially when it’s used in for-loops (--i/i-- too)

Saves 6 gas PER LOOP

Instances:

contracts/ethregistrar/ETHRegistrarController.sol:256:
contracts/ethregistrar/StringUtils.sol:14:
contracts/dnssec-oracle/BytesUtils.sol:266:
contracts/dnssec-oracle/BytesUtils.sol:313:
contracts/dnssec-oracle/DNSSECImpl.sol:93:

contracts/ethregistrar/ETHRegistrarController.sol:256:        for (uint256 i = 0; i < data.length; i++) {
contracts/ethregistrar/StringUtils.sol:14:        for(len = 0; i < bytelength; len++) {
contracts/dnssec-oracle/BytesUtils.sol:266:        for(uint i = 0; i < len; i++) {
contracts/dnssec-oracle/BytesUtils.sol:313:        for(uint256 idx = off; idx < off + len; idx++) {
contracts/dnssec-oracle/DNSSECImpl.sol:93:        for(uint i = 0; i < input.length; i++) {

Reference:

https://code4rena.com/reports/2022-05-cally#g-19-i-costs-less-gas-than-i-especially-when-its-used-in-for-loops---ii---too

4. Using bools for storage incurs overhead

Booleans are more expensive than uint256 or any type that takes up a full word because each write operation emits an extra SLOAD to first read the slot's contents, replace the bits taken up by the boolean, and then write back. This is the compiler's defense against contract upgrades and pointer aliasing, and it cannot be disabled.

https://github.com/OpenZeppelin/openzeppelin-contracts/blob/58f635312aa21f947cae5f8578638a85aa2519f5/contracts/security/ReentrancyGuard.sol#L23-L27 Use uint256(1) and uint256(2) for true/false to avoid a Gwarmaccess (100 gas) for the extra SLOAD, and to avoid Gsset (20000 gas) when changing from ‘false’ to ‘true’, after having been ‘true’ in the past

There are 2 instances of this issue:
contracts/wrapper/Controllable.sol:7:
contracts/wrapper/ERC1155Fuse.sol:19:

contracts/wrapper/Controllable.sol:7:    mapping(address=>bool) public controllers;
contracts/wrapper/ERC1155Fuse.sol:19:    mapping(address => mapping(address => bool)) private _operatorApprovals;

5. abi.encode() is less efficient than abi.encodePacked()

There are 1 instances of this issue:
contracts/ethregistrar/ETHRegistrarController.sol:106:

contracts/ethregistrar/ETHRegistrarController.sol:106:                abi.encode(

Reference:

https://code4rena.com/reports/2022-06-notional-coop#15-abiencode-is-less-efficient-than-abiencodepacked

6. Custom Errors instead of Revert Strings to save Gas

Custom errors from Solidity 0.8.4 are cheaper than revert strings (cheaper deployment cost and runtime cost when the revert condition is met)

Starting from Solidity v0.8.4,there is a convenient and gas-efficient way to explain to users why an operation failed through the use of custom errors. Until now, you could already use strings to give more information about failures (e.g., revert("Insufficient funds.");),but they are rather expensive, especially when it comes to deploy cost, and it is difficult to use dynamic information in them.

Custom errors are defined using the error statement, which can be used inside and outside of contracts (including interfaces and libraries).

Instances:

RRUtils.sol:
307

ETHRegistrarController.sol:
99-102
137-140
196-199
232-235
238-241
242
259-262

ReverseRegistrar.sol
41-47
52-56

ByteUtil.sol
28
42

Controllable.sol
17

ERC1155Fuse.sol
60-63
85-88
107-110
176
177-180
195-198
199
200-203
215-218
248
249
250-253
290-293

References:

https://blog.soliditylang.org/2021/04/21/custom-errors/

Remediation:

I suggest replacing revert strings with custom errors.

7. Reduce the size of error messages (Long revert Strings)

Shortening revert strings to fit in 32 bytes will decrease deployment time gas and will decrease runtime gas when the revert condition is met.

Revert strings that are longer than 32 bytes require at least one additional mstore, along with additional overhead for computing memory offset, etc.

Instances:

ETHRegistrarController.sol:
99-102
137-140
196-199
232-235
238-241
242
259-262

ReverseRegistrar.sol
41-47
52-56

Controllable.sol
17

ERC1155Fuse.sol
60-63
85-88
107-110
176
177-180
195-198
199
200-203
215-218
249
250-253
290-293

I suggest shortening the revert strings to fit in 32 bytes, or that using custom errors as described next.****

Custom errors from Solidity 0.8.4 are cheaper than revert strings
(cheaper deployment cost and runtime cost when the revert condition is
met)

Source Custom Errors in Solidity:

8. Variables: 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.

We can use uint number; instead of uint number = 0;

Instances:

contracts/ethregistrar/ETHRegistrarController.sol:256:        for (uint256 i = 0; i < data.length; i++) {
contracts/ethregistrar/StringUtils.sol:12:        uint i = 0;
contracts/dnssec-oracle/BytesUtils.sol:56:        for (uint idx = 0; idx < shortest; idx += 32) {
contracts/dnssec-oracle/BytesUtils.sol:264:        uint ret = 0;
contracts/dnssec-oracle/BytesUtils.sol:266:        for(uint i = 0; i < len; i++) {
contracts/dnssec-oracle/DNSSECImpl.sol:93:        for(uint i = 0; i < input.length; i++) {
contracts/dnssec-oracle/RRUtils.sol:50:        uint count = 0;
contracts/dnssec-oracle/RRUtils.sol:63:    uint constant RRSIG_TYPE = 0;
contracts/dnssec-oracle/RRUtils.sol:181:    uint constant DNSKEY_FLAGS = 0;
contracts/dnssec-oracle/RRUtils.sol:200:    uint constant DS_KEY_TAG = 0;
contracts/dnssec-oracle/RRUtils.sol:310:            for(uint i = 0; i < data.length + 31; i += 32) {
contracts/wrapper/INameWrapper.sol:16:uint32 constant CAN_DO_EVERYTHING = 0;
contracts/wrapper/ERC1155Fuse.sol:92:        for (uint256 i = 0; i < accounts.length; ++i) {
contracts/wrapper/ERC1155Fuse.sol:205:        for (uint256 i = 0; i < ids.length; ++i) {

The text was updated successfully, but these errors were encountered:

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

Sm4rty issue #249

00ab037

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Gas Optimizations #249

Gas Optimizations #249

code423n4 commented Jul 19, 2022

Gas Optimizations #249

Gas Optimizations #249

Comments

code423n4 commented Jul 19, 2022

1. An array’s length should be cached to save gas in for-loops

Instances:

Remediation:

2. Require instead of &&

Instances include:

Mitigation:

3. ++i costs less gas than i++, especially when it’s used in for-loops (--i/i-- too)

Instances:

Reference:

4. Using bools for storage incurs overhead

5. abi.encode() is less efficient than abi.encodePacked()

Reference:

6. Custom Errors instead of Revert Strings to save Gas

Instances:

References:

Remediation:

7. Reduce the size of error messages (Long revert Strings)

Instances:

8. Variables: No need to explicitly initialize variables with default values

Instances: