[G-01] abi.encode() is less efficient than abi.encodePacked()
Impact
Consider using abi.encodePacked() instead for efficieny.
Findings:
contracts/gateway/L1GraphTokenGateway.sol:L249 return abi.encode(seqNum);
contracts/gateway/L1GraphTokenGateway.sol:L342 abi.encode(emptyBytes, _data)
contracts/l2/token/GraphTokenUpgradeable.sol:L88 abi.encode(PERMIT_TYPEHASH, _owner, _spender, _value, nonces[_owner], _deadline)
contracts/l2/token/GraphTokenUpgradeable.sol:L162 abi.encode(
contracts/l2/gateway/L2GraphTokenGateway.sol:L174 return abi.encode(id);
contracts/l2/gateway/L2GraphTokenGateway.sol:L275 abi.encode(0, _data) // we don't need to track exitNums (b/c we have no fast exits) so we always use 0
[G-02] Use assembly to check for zero address.
Impact
Save 6 gas when assembly is used to check for zero address.
e.g:
contracts/upgrades/GraphProxy.sol:L105 require(_newAdmin != address(0), "Cannot change the admin of a proxy to the zero address");
[G-03] Using bools for storage incurs overhead.
Impact
// 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.
[G-04] Use custom errors rather than revert()/require() string to save gas
Impact
Custom errors are available from solidity version 0.8.4, it saves around 50 gas each time they are hit by avoiding having to allocate and store the revert string.
[G-05] Public functions not called by the contract should be declared external instead
Impact
public functions that are never called by the contract should be declared external to save gas.
Findings:
contracts/upgrades/GraphProxyAdmin.sol:L68 function changeProxyAdmin(IGraphProxy _proxy, address _newAdmin) public onlyGovernor {
contracts/upgrades/GraphProxyAdmin.sol:L77 function upgrade(IGraphProxy _proxy, address _implementation) public onlyGovernor {
[G-06] Using > 0 costs more gas than != 0 when used on a uint in a require() statement.
Impact
When working with unsigned integer types, comparisons with != 0 are cheaper than with > 0 . This changes saves 6 gas per instance.
Use a solidity version of at least 0.8.2 to get simple compiler automatic inlining
Use a solidity version of at least 0.8.3 to get better struct packing and cheaper multiple storage reads
Use a solidity version of at least 0.8.4 to get custom errors, which are cheaper at deployment than revert()/require() strings
Use a solidity version of at least 0.8.10 to have external calls skip contract existence checks if the external call has a return value.
[G-08] Functions guaranteed to revert when called by normal users can be declared as payable.
Impact
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. 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.
Findings:
contracts/gateway/L1GraphTokenGateway.sol:L99 function initialize(address _controller) external onlyImpl {
contracts/gateway/L1GraphTokenGateway.sol:L109 function setArbitrumAddresses(address _inbox, address _l1Router) external onlyGovernor {
contracts/gateway/L1GraphTokenGateway.sol:L121 function setL2TokenAddress(address _l2GRT) external onlyGovernor {
contracts/gateway/L1GraphTokenGateway.sol:L131 function setL2CounterpartAddress(address _l2Counterpart) external onlyGovernor {
contracts/gateway/L1GraphTokenGateway.sol:L141 function setEscrowAddress(address _escrow) external onlyGovernor {
contracts/gateway/L1GraphTokenGateway.sol:L152 function addToCallhookWhitelist(address _newWhitelisted) external onlyGovernor {
contracts/gateway/L1GraphTokenGateway.sol:L164 function removeFromCallhookWhitelist(address _notWhitelisted) external onlyGovernor {
contracts/gateway/GraphTokenGateway.sol:L30 function setPauseGuardian(address _newPauseGuardian) external onlyGovernor {
contracts/gateway/GraphTokenGateway.sol:L47 function setPaused(bool _newPaused) external onlyGovernorOrGuardian {
contracts/gateway/BridgeEscrow.sol:L20 function initialize(address _controller) external onlyImpl {
contracts/gateway/BridgeEscrow.sol:L28 function approveAll(address _spender) external onlyGovernor {
contracts/gateway/BridgeEscrow.sol:L36 function revokeAll(address _spender) external onlyGovernor {
contracts/upgrades/GraphUpgradeable.sol:L50 function acceptProxy(IGraphProxy _proxy) external onlyProxyAdmin(_proxy) {
contracts/upgrades/GraphProxyAdmin.sol:L68 function changeProxyAdmin(IGraphProxy _proxy, address _newAdmin) public onlyGovernor {
contracts/upgrades/GraphProxyAdmin.sol:L77 function upgrade(IGraphProxy _proxy, address _implementation) public onlyGovernor {
contracts/upgrades/GraphProxyAdmin.sol:L86 function acceptProxy(GraphUpgradeable _implementation, IGraphProxy _proxy) public onlyGovernor {
contracts/governance/Governed.sol:L40 function transferOwnership(address _newGovernor) external onlyGovernor {
contracts/governance/Managed.sol:L95 function setController(address _controller) external onlyController {
contracts/l2/token/L2GraphToken.sol:L48 function initialize(address _owner) external onlyImpl {
contracts/l2/token/L2GraphToken.sol:L59 function setGateway(address _gw) external onlyGovernor {
contracts/l2/token/L2GraphToken.sol:L69 function setL1Address(address _addr) external onlyGovernor {
contracts/l2/token/L2GraphToken.sol:L80 function bridgeMint(address _account, uint256 _amount) external override onlyGateway {
contracts/l2/token/L2GraphToken.sol:L90 function bridgeBurn(address _account, uint256 _amount) external override onlyGateway {
contracts/l2/token/GraphTokenUpgradeable.sol:L105 function addMinter(address _account) external onlyGovernor {
contracts/l2/token/GraphTokenUpgradeable.sol:L114 function removeMinter(address _account) external onlyGovernor {
contracts/l2/token/GraphTokenUpgradeable.sol:L132 function mint(address _to, uint256 _amount) external onlyMinter {
contracts/l2/gateway/L2GraphTokenGateway.sol:L87 function initialize(address _controller) external onlyImpl {
contracts/l2/gateway/L2GraphTokenGateway.sol:L97 function setL2Router(address _l2Router) external onlyGovernor {
contracts/l2/gateway/L2GraphTokenGateway.sol:L107 function setL1TokenAddress(address _l1GRT) external onlyGovernor {
contracts/l2/gateway/L2GraphTokenGateway.sol:L117 function setL1CounterpartAddress(address _l1Counterpart) external onlyGovernor {
[G-09] Revert message greater than 32 bytes
Impact
Keep revert message lower than or equal to 32 bytes to save gas.
Findings:
contracts/gateway/GraphTokenGateway.sol:L19 require(
contracts/upgrades/GraphProxy.sol:L105 require(_newAdmin != address(0), "Cannot change the admin of a proxy to the zero address");
contracts/upgrades/GraphProxy.sol:L141 require(Address.isContract(_pendingImplementation), "Implementation must be a contract");
contracts/upgrades/GraphProxy.sol:L142 require(
contracts/upgrades/GraphUpgradeable.sol:L32 require(msg.sender == _implementation(), "Caller must be the implementation");
contracts/governance/Managed.sol:L53 require(msg.sender == controller.getGovernor(), "Caller must be Controller governor");
[G-10] Splitting require() statements that use && saves gas.
Impact
Consider splitting the require() statements to save gas.
[G-01]
abi.encode()
is less efficient thanabi.encodePacked()
Impact
Consider using
abi.encodePacked()
instead for efficieny.Findings:
[G-02] Use assembly to check for zero address.
Impact
Save 6 gas when assembly is used to check for zero address. e.g:
Findings:
[G-03] Using bools for storage incurs overhead.
Impact
https://github.com/OpenZeppelin/openzeppelin-contracts/blob/58f635312aa21f947cae5f8578638a85aa2519f5/contracts/security/ReentrancyGuard.sol#L23-L27 Use
uint256(1)
anduint256(2)
for true/false to avoid a Gwarmaccess (100 gas), and to avoid Gsset (20000 gas) when changing from ‘false’ to ‘true’, after having been ‘true’ in the pastFindings:
[G-04] Use custom errors rather than
revert()
/require()
string to save gasImpact
Custom errors are available from solidity version 0.8.4, it saves around 50 gas each time they are hit by avoiding having to allocate and store the revert string.
Findings:
[G-05] Public functions not called by the contract should be declared external instead
Impact
public functions that are never called by the contract should be declared external to save gas.
Findings:
[G-06] Using
> 0
costs more gas than!= 0
when used on a uint in arequire()
statement.Impact
When working with unsigned integer types, comparisons with != 0 are cheaper than with > 0 . This changes saves 6 gas per instance.
Findings:
[G-07] Use a more recent version of solidity.
Impact
Use a solidity version of at least 0.8.2 to get simple compiler automatic inlining Use a solidity version of at least 0.8.3 to get better struct packing and cheaper multiple storage reads Use a solidity version of at least 0.8.4 to get custom errors, which are cheaper at deployment than revert()/require() strings Use a solidity version of at least 0.8.10 to have external calls skip contract existence checks if the external call has a return value.
Findings:
[G-08] Functions guaranteed to revert when called by normal users can be declared as payable.
Impact
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. 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.
Findings:
[G-09] Revert message greater than 32 bytes
Impact
Keep revert message lower than or equal to 32 bytes to save gas.
Findings:
[G-10] Splitting
require()
statements that use && saves gas.Impact
Consider splitting the
require()
statements to save gas.Findings: