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

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

Gas Optimizations #316

code423n4 opened this issue Jul 19, 2022 · 0 comments

Comments

@code423n4
Copy link
Contributor

Returns variable ignored

Summary

Not using the named return variables when a function returns wastes deployment gas

Details

There are functions that includes returns(type variableName)
and then doesn't use that variableName that had been initialised.
This consumes gas.

Github permalinks

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

Mitigation

returns variable ignored wastes extra gas, remove them or assigned them

use of custom errors rather than revert() / require()

summary

Custom errors reduce 38 gas if the condition is met and 22 gas otherwise.
Also reduces contract size and deployment costs.

details

Since version 0.8.4 the use of custom errors rather than revert() / require() saves gas as noticed in
https://blog.soliditylang.org/2021/04/21/custom-errors/
code-423n4/2022-04-pooltogether-findings#13

github permalinks

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/wrapper/Controllable.sol#L16-L20
https://github.com/code-423n4/2022-07-ens/blob/main/contracts/wrapper/ERC1155Fuse.sol#L60-L63
https://github.com/code-423n4/2022-07-ens/blob/main/contracts/wrapper/ERC1155Fuse.sol#L85-L88
https://github.com/code-423n4/2022-07-ens/blob/main/contracts/wrapper/ERC1155Fuse.sol#L107-L110
https://github.com/code-423n4/2022-07-ens/blob/main/contracts/wrapper/ERC1155Fuse.sol#L176
https://github.com/code-423n4/2022-07-ens/blob/main/contracts/wrapper/ERC1155Fuse.sol#L177-L180
https://github.com/code-423n4/2022-07-ens/blob/main/contracts/wrapper/ERC1155Fuse.sol#L195-L198
https://github.com/code-423n4/2022-07-ens/blob/main/contracts/wrapper/ERC1155Fuse.sol#L199
https://github.com/code-423n4/2022-07-ens/blob/main/contracts/wrapper/ERC1155Fuse.sol#L200-L203
https://github.com/code-423n4/2022-07-ens/blob/main/contracts/wrapper/ERC1155Fuse.sol#L215-L218
https://github.com/code-423n4/2022-07-ens/blob/main/contracts/wrapper/ERC1155Fuse.sol#L248
https://github.com/code-423n4/2022-07-ens/blob/main/contracts/wrapper/ERC1155Fuse.sol#L249
https://github.com/code-423n4/2022-07-ens/blob/main/contracts/wrapper/ERC1155Fuse.sol#L250-L253
https://github.com/code-423n4/2022-07-ens/blob/main/contracts/wrapper/ERC1155Fuse.sol#L290-L293
https://github.com/code-423n4/2022-07-ens/blob/main/contracts/wrapper/BytesUtil.sol#L28
https://github.com/code-423n4/2022-07-ens/blob/main/contracts/wrapper/BytesUtil.sol#L42
https://github.com/code-423n4/2022-07-ens/blob/main/contracts/ethregistrar/ETHRegistrarController.sol#L99-L102
https://github.com/code-423n4/2022-07-ens/blob/main/contracts/ethregistrar/ETHRegistrarController.sol#L137-L140
https://github.com/code-423n4/2022-07-ens/blob/main/contracts/ethregistrar/ETHRegistrarController.sol#L196-L199
https://github.com/code-423n4/2022-07-ens/blob/main/contracts/ethregistrar/ETHRegistrarController.sol#L232-L235
https://github.com/code-423n4/2022-07-ens/blob/main/contracts/ethregistrar/ETHRegistrarController.sol#L238-L241
https://github.com/code-423n4/2022-07-ens/blob/main/contracts/ethregistrar/ETHRegistrarController.sol#L242
https://github.com/code-423n4/2022-07-ens/blob/main/contracts/ethregistrar/ETHRegistrarController.sol#L259-L262

mitigation

replace each require by an error

use of > 0 costs more than != 0 on uints

summary

using > 0 costs 6 more gas than != 0 when used on a require() statement as negative numbers are not allowed in uint values

details

uint variables can't go negative so it can be improved the gas

github permalinks

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/wrapper/BytesUtil.sol#L44
https://github.com/code-423n4/2022-07-ens/blob/main/contracts/dnssec-oracle/RRUtils.sol#L245
https://github.com/code-423n4/2022-07-ens/blob/main/contracts/ethregistrar/ETHRegistrarController.sol#L98

mitigation

replace > 0 to != 0 for extra gas savings by each time is called the condition

duplicated require() check should be refactored

summary

duplicated require() / revert() checks should be
refactored to a modifier or function to save gas

details

Event appears twice and can be reduced

github permalinks

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

mitigation

refactor this checks to different functions to save gas

Index initialised in for loop

summary

for loops doens't need to initialize loop indexes to 0 as it is the default uint value
this saves gas

details

Multiple initializations on loop's index to 0

github permalinks

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/wrapper/ERC1155Fuse.sol#L92
https://github.com/code-423n4/2022-07-ens/blob/main/contracts/wrapper/ERC1155Fuse.sol#L205
https://github.com/code-423n4/2022-07-ens/blob/main/contracts/ethregistrar/StringUtils.sol#L14
https://github.com/code-423n4/2022-07-ens/blob/main/contracts/dnssec-oracle/BytesUtils.sol#L56
https://github.com/code-423n4/2022-07-ens/blob/main/contracts/dnssec-oracle/BytesUtils.sol#L266
https://github.com/code-423n4/2022-07-ens/blob/main/contracts/dnssec-oracle/BytesUtils.sol#L313
https://github.com/code-423n4/2022-07-ens/blob/main/contracts/dnssec-oracle/RRUtils.sol#L310
https://github.com/code-423n4/2022-07-ens/blob/main/contracts/ethregistrar/ETHRegistrarController.sol#L256

mitigation

Don't initialise the index

variable initialised without need

summary

The default value of the uints is 0

details

Not initializing to 0 explicitly saves gas on variable declarations

github permalinks

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/ethregistrar/StringUtils.sol#L12
https://github.com/code-423n4/2022-07-ens/blob/main/contracts/wrapper/INameWrapper.sol#L16
https://github.com/code-423n4/2022-07-ens/blob/main/contracts/dnssec-oracle/BytesUtils.sol#L264
https://github.com/code-423n4/2022-07-ens/blob/main/contracts/dnssec-oracle/RRUtils.sol#L50
https://github.com/code-423n4/2022-07-ens/blob/main/contracts/dnssec-oracle/RRUtils.sol#L63
https://github.com/code-423n4/2022-07-ens/blob/main/contracts/dnssec-oracle/RRUtils.sol#L181
https://github.com/code-423n4/2022-07-ens/blob/main/contracts/dnssec-oracle/RRUtils.sol#L200

mitigation

Don't initialise uint variables to 0

variables in a struct should be ordered by same data type to improve gas and storage

summary

the order in structs affects memory and gas usage

details

there are several structs with no order on the variable types

github permalinks

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

mitigation

put together all the same data types

use of counter++ in loop rather than ++counter

summary

++i costs less gas than i++, especially when it's used in for loops

details

using ++i doesn't affect the flow of regular for loops and improves gas cost

github permalinks

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/ethregistrar/StringUtils.sol#L12
https://github.com/code-423n4/2022-07-ens/blob/main/contracts/dnssec-oracle/BytesUtils.sol#L266
https://github.com/code-423n4/2022-07-ens/blob/main/contracts/dnssec-oracle/BytesUtils.sol#L313
https://github.com/code-423n4/2022-07-ens/blob/main/contracts/ethregistrar/ETHRegistrarController.sol#L256

mitigation

Substitute to ++variableName when posible

use of var-- where it can be substitute by --var

summary

--i costs less gas than i--

details

i-- is used within a while loop and can be optimised to --i

github permalinks

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

mitigation

replace i-- to --i

unchecked index in loop

summary

unchecked operations as the ++i on for loops are cheaper than checked one.
If they are not supposed to reach the maximum value, they don't need to check for overflow

details

Gas can be optimised

github permalinks

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/wrapper/ERC1155Fuse.sol#L92-L94
https://github.com/code-423n4/2022-07-ens/blob/main/contracts/wrapper/ERC1155Fuse.sol#L205-L220
https://github.com/code-423n4/2022-07-ens/blob/main/contracts/ethregistrar/StringUtils.sol#L14-L31
https://github.com/code-423n4/2022-07-ens/blob/main/contracts/dnssec-oracle/BytesUtils.sol#L266-L275
https://github.com/code-423n4/2022-07-ens/blob/main/contracts/ethregistrar/ETHRegistrarController.sol#L256-L267

mitigation

add unchecked ++i at the end of all the for loop where it's not expected to overflow and remove them from the for header

loop length stored in local variable for performance

summary

loop length variables assigned to a local variable improves gas usage

details

loop length is used without a uint variable

github permalinks

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

mitigation

assign lengths to local variable for loops or when they are used several times

variable should be declared constant

summary

variables that can be constant should be constant for gas usage

details

variable is only assigned 1 so should be constant

github permalinks

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

mitigation

assigned it to constant

Functions guaranteed to revert when called by normal users can be marked payable

Summary

If a function modifier such as onlyOwner is used, the function will revert if a normal user tries to pay the function.

Marking the function as payable will lower the gas cost for legitimate callers because the compiler will not include checks for whether a payment was provided.

Details

The extra opcodes avoided are:
CALLVALUE (2), DUP1 (3), ISZERO (3), PUSH2 (3), JUMPI (10), PUSH1 (3), DUP1 (3), REVERT(0), JUMPDEST (1), POP (2), which costs an average of about 21 gas per call to the function, in addition to the extra deployment cost

Github permalinks

https://github.com/code-423n4/2022-07-ens/blob/main/contracts/wrapper/Controllable.sol#L11-L14
https://github.com/code-423n4/2022-07-ens/blob/main/contracts/dnssec-oracle/Owned.sol#L18-L20

Mitigation

It's suggested to add payable to functions guaranteed to revert when called by normal users to improve gas costs

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
Projects
None yet
Development

No branches or pull requests

1 participant