search

code-423n4 / 2022-10-thegraph-findings

0 stars 0 forks source link

Gas Optimizations #194

Open code423n4 opened 1 year ago

code423n4 commented 1 year ago

Using bools for storage incurs overhead

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 falseto true, after having been truein the past

File: contracts/governance/Pausable.sol   #1
8    bool internal _partialPaused;

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

File: contracts/l2/token/GraphTokenUpgradeable.sol   #1
51    mapping(address => bool) private _minters;
52    mapping(address => uint256) public nonces;

REQUIRE() STATEMENTS SHOULD HAVE DESCRIPTIVE REASON STRINGS

File: contracts/upgrades/GraphProxyAdmin.sol   #1
47        require(success);

Using > 0 costs more gas than != 0 when used on a uint in a require() statement

This change saves 6 gas per instance

File: contracts/l2/gateway/L2GraphTokenGateway.sol   #1
146        require(_amount > 0, "INVALID_ZERO_AMOUNT");

Splitting require() statements that use && saves gas

Instead of using the && operator in a single require statement to check multiple conditions, I suggest using multiple require statements with 1 condition per require statement (saving 3 gas per &):

File: contracts/governance/Governed.sol   #1
55            pendingGovernor != address(0) && msg.sender == pendingGovernor,

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

File: contracts/l2/token/GraphTokenUpgradeable.sol   #1
88                    abi.encode(PERMIT_TYPEHASH, _owner, _spender, _value, nonces[_owner], _deadline)

Inline a modifier that's only used once

As onlyGovernor()is only used once in this contract (in function transferOwnership()), it should get inlined to save gas without losing readability:

As onlyMinter()is only used once in this contract (in function mint()), it should get inlined to save gas without losing readability:

Use custom errors rather than revert()/require() strings to save deployment gas

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

Custom errors save ~50 gas each time they’re hit by avoiding having to allocate and store the revert string. Not defining the strings also save

For example:         File: contracts/governance/Governed.sol
24        require(msg.sender == governor, "Only Governor can call");