search

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

0 stars 0 forks source link

Gas Optimizations #274

Open code423n4 opened 1 year ago

code423n4 commented 1 year ago

Index

  1. Call to KECCAK256 should use IMMUTABLE rather than constant
  2. Require instead of &&
  3. != 0 is cheaper than > 0 especially in state variables
  4. Require / Revert strings longer than 32 bytes cost extra gas
  5. Use custom errors rather than REVERT()/REQUIRE() strings to save deployment gas
  6. Don't compare boolean expressions to boolean literals
  7. Using bools for storage incurs overhead
  8. ABI.ENCODE() is less efficient than ABI.ENCODEPACKED()
  9. Use a more recent version of solidity
  10. Cache storage variables in local variables
  11. Internal functions used once could be inlined

Details

1. Call to KECCAK256 should use IMMUTABLE rather than constant

Description

Expressions for constant values such as a call to KECCAK256 should use IMMUTABLE rather than constant

Lines in the code

GraphTokenUpgradeable.sol#L38 GraphTokenUpgradeable.sol#L39

2. Require instead of &&

Description

Split of conditions of an require sentence in different requires sentences can save gas

Lines in the code

L1GraphTokenGateway.sol#L142

3. != 0 is cheaper than > 0 especially in state variables

Description

Replace all > 0 for != 0. In cases where we used a variable from state assigned to a local variable it's the same gas save.

Proof of concept

contract TestA {
    uint256 _paramA;

    function Test () public returns (bool) {
        return _paramA > 0;
    }

}

contract TestB {
    uint256 _paramA;

    function Test () public returns (bool) {
        return _paramA != 0;
    }

}

contract TestC {
    uint256 _paramA;

    function Test () public returns (bool) {
        uint256 aux = _paramA;
        return aux > 0;
    }

}

contract TestD {
    uint256 _paramA;

    function Test () public returns (bool) {
        uint256 aux = _paramA;
        return aux != 0;
    }

}

Lines in the code

L2GraphTokenGateway.sol#L146 L2GraphTokenGateway.sol#L238 L1GraphTokenGateway.sol#L201 L1GraphTokenGateway.sol#L217

4. Require / Revert strings longer than 32 bytes cost extra gas

Description

Each extra memory word of bytes past the original 32 incurs an MSTORE which costs 3 gas

Lines in the code

GraphUpgradeable.sol#L32 GraphProxy.sol#L105 GraphProxy.sol#L141 GraphProxy.sol#L142 Managed.sol#L53 GraphTokenGateway.sol#L19

5. Use custom errors rather than REVERT()/REQUIRE() strings to save deployment gas

Description

Custom errors are available from solidity version 0.8.4. Custom errors save ~50 gas each time they're hitby avoiding having to allocate and store the revert string. Not defining the strings also save deployment gas.

Lines in the code

GraphUpgradeable.sol#L24 GraphUpgradeable.sol#L32 Governed.sol#L24 Governed.sol#L41 Governed.sol#L54 L2GraphToken.sol#L36 L2GraphToken.sol#L49 L2GraphToken.sol#L60 L2GraphToken.sol#L70 GraphProxyAdmin.sol#L34 GraphProxyAdmin.sol#L47 GraphProxyAdmin.sol#L59 GraphProxyStorage.sol#L62 GraphProxy.sol#L105 GraphProxy.sol#L133 GraphProxy.sol#L141 GraphProxy.sol#L142 GraphProxy.sol#L157 Managed.sol#L44 Managed.sol#L45 Managed.sol#L49 Managed.sol#L53 Managed.sol#L57 Managed.sol#L104 GraphTokenUpgradeable.sol#L60 GraphTokenUpgradeable.sol#L94 GraphTokenUpgradeable.sol#L95 GraphTokenUpgradeable.sol#L106 GraphTokenUpgradeable.sol#L115 GraphTokenUpgradeable.sol#L123 L2GraphTokenGateway.sol#L69 L2GraphTokenGateway.sol#L98 L2GraphTokenGateway.sol#L108 L2GraphTokenGateway.sol#L118 L2GraphTokenGateway.sol#L126 L2GraphTokenGateway.sol#L145 L2GraphTokenGateway.sol#L146 L2GraphTokenGateway.sol#L147 L2GraphTokenGateway.sol#L148 L2GraphTokenGateway.sol#L153 L2GraphTokenGateway.sol#L233 L2GraphTokenGateway.sol#L234 L1GraphTokenGateway.sol#L74 L1GraphTokenGateway.sol#L78 L1GraphTokenGateway.sol#L82 L1GraphTokenGateway.sol#L110 L1GraphTokenGateway.sol#L111 L1GraphTokenGateway.sol#L122 L1GraphTokenGateway.sol#L132 L1GraphTokenGateway.sol#L142 L1GraphTokenGateway.sol#L153 L1GraphTokenGateway.sol#L154 L1GraphTokenGateway.sol#L165 L1GraphTokenGateway.sol#L166 L1GraphTokenGateway.sol#L200 L1GraphTokenGateway.sol#L201 L1GraphTokenGateway.sol#L202 L1GraphTokenGateway.sol#L213 L1GraphTokenGateway.sol#L217 L1GraphTokenGateway.sol#L224 L1GraphTokenGateway.sol#L271 L1GraphTokenGateway.sol#L275 GraphTokenGateway.sol#L19 GraphTokenGateway.sol#L31 GraphTokenGateway.sol#L40

6. Don't compare boolean expressions to boolean literals

Description

The following conditions could be represented in a way that save gas.

Example,

Proof of concept

contract TestA {
    function Test (bool paramA) public {
        require(paramA == true, "Test bool");
    }
}

contract TestB {
    function Test (bool paramA) public {
        require(paramA, "Test bool");
    }
}

Lines in the code

L1GraphTokenGateway.sol#L214

7. Using bools for storage incurs overhead

Description

Use uint256(1) and uint256(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 past

Lines in the code

Pausable.sol#L8 Pausable.sol#L10 GraphProxyAdmin.sol#L33 GraphProxyAdmin.sol#L46 GraphProxyAdmin.sol#L58 GraphProxy.sol#L132 GraphTokenUpgradeable.sol#L51 L1GraphTokenGateway.sol#L35

8. ABI.ENCODE() is less efficient than ABI.ENCODEPACKED()

Description

abi.encode will apply ABI encoding rules. Therefore all elementary types are padded to 32 bytes and dynamic arrays include their length. Therefore it is possible to also decode this data again (with abi.decode) when the type are known.

abi.encodePacked will only use the only use the minimal required memory to encode the data. E.g. an address will only use 20 bytes and for dynamic arrays only the elements will be stored without length. For more info see the Solidity docs for packed mode

For the input of the keccak method it is important that you can ensure that the resulting bytes of the encoding are unique. So if you always encode the same types and arrays always have the same length then there is no problem. But if you switch the parameters that you encode or encode multiple dynamic arrays you might have conflicts.

For example:

abi.encodePacked(address(0x0000000000000000000000000000000000000001), uint(0)) encodes to the same as abi.encodePacked(uint(0x0000000000000000000000000000000000000001000000000000000000000000), address(0))

and

abi.encodePacked(uint, uint) encodes to the same as abi.encodePacked(uint, uint)

Therefore these examples will also generate the same hashes even so they are different inputs.

On the other hand you require less memory and therefore in most cases abi.encodePacked uses less gas than abi.encode.

Lines in the code

GraphTokenUpgradeable.sol#L88 GraphTokenUpgradeable.sol#L162 L2GraphTokenGateway.sol#L174 L2GraphTokenGateway.sol#L275 L1GraphTokenGateway.sol#L249 L1GraphTokenGateway.sol#L342

9. Use a more recent version of solidity

Description

Use a solidity version of at least 0.8.0 to get overflow protection without SafeMath Use a solidity version of at least 0.8.2 to get 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.

Lines in the code

BridgeEscrow.sol#L3 GraphUpgradeable.sol#L3 Governed.sol#L3 Pausable.sol#L3 L2GraphToken.sol#L3 GraphProxyAdmin.sol#L3 GraphProxyStorage.sol#L3 GraphProxy.sol#L3 Managed.sol#L3 GraphTokenUpgradeable.sol#L3 L2GraphTokenGateway.sol#L3 L1GraphTokenGateway.sol#L3 GraphTokenGateway.sol#L3 IGraphCurationToken.sol#L3 ICallhookReceiver.sol#L9 IGraphProxy.sol#L3 IEpochManager.sol#L3 IController.sol#L3 IGraphToken.sol#L3 IRewardsManager.sol#L3 IStakingData.sol#L3 ICuration.sol#L3 IStaking.sol#L3

10. Cache storage variables in local variables

Description

To avoid access to the storage to save gas is better to store the value in a local variable and use it.

Governed.acceptOwnership

    function acceptOwnership() external {
+       address public _pendingGovernor = pendingGovernor;
        require(
-           pendingGovernor != address(0) && msg.sender == pendingGovernor,
+           _pendingGovernor != address(0) && msg.sender == _pendingGovernor,           
            "Caller must be pending governor"
        );

        address oldGovernor = governor;
-       address oldPendingGovernor = pendingGovernor;

-       governor = pendingGovernor;
+       governor = _pendingGovernor;
        pendingGovernor = address(0);

        emit NewOwnership(oldGovernor, governor);
-       emit NewPendingOwnership(oldPendingGovernor, pendingGovernor);
+       emit NewPendingOwnership(_pendingGovernor, pendingGovernor);
    }

Lines in the code

Governed.sol#L53-L67

11. Internal functions used once could be inlined

Managed._notPartialPaused

Managed.sol#L43

Managed._onlyGovernor

Managed.sol#L52

Managed._onlyController

Managed.sol#L56