Gas Optimizations

[code423n4]

Code refactoring

The chain of getOwnBalance(), _executeSwaps(), getOwnBalance(), require(), assign, should be refactored to a common function https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/NXTPFacet.sol#L91-L100 https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/HopFacet.sol#L102-L111 https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/AnyswapFacet.sol#L85-L94 https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/CBridgeFacet.sol#L98-L107 The same should be done for the native versions

Don't compare boolean expressions to boolean literals

if (<x> == true) => if (<x>), if (<x> == false) => if (!<x>)

1. File: src/Facets/DexManagerFacet.sol (line 20)

       if (s.dexWhitelist[_dex] == true) {

2. File: src/Facets/DexManagerFacet.sol (line 34)

           if (s.dexWhitelist[_dexs[i]] == true) {

3. File: src/Facets/DexManagerFacet.sol (line 47)

       if (s.dexWhitelist[_dex] == false) {

4. File: src/Facets/DexManagerFacet.sol (line 66)

           if (s.dexWhitelist[_dexs[i]] == false) {

5. File: src/Facets/Swapper.sol (line 16)

               ls.dexWhitelist[_swapData[i].approveTo] == true && ls.dexWhitelist[_swapData[i].callTo] == true,

6. File: src/Facets/Swapper.sol (line 16)

               ls.dexWhitelist[_swapData[i].approveTo] == true && ls.dexWhitelist[_swapData[i].callTo] == true,

Expressions for constant values such as a call to keccak256(), should use immutable rather than constant

See this issue for a detail description of the issue

1. File: src/Libraries/LibDiamond.sol (line 7)

   bytes32 internal constant DIAMOND_STORAGE_POSITION = keccak256("diamond.standard.diamond.storage");

2. File: src/Facets/NXTPFacet.sol (line 18)

   bytes32 internal constant NAMESPACE = keccak256("com.lifi.facets.nxtp");

3. File: src/Facets/HopFacet.sol (line 18)

   bytes32 internal constant NAMESPACE = keccak256("com.lifi.facets.hop");

4. File: src/Facets/CBridgeFacet.sol (line 18)

   bytes32 internal constant NAMESPACE = keccak256("com.lifi.facets.cbridge2");

Superfluous event fields

block.timestamp and block.number are added to event information by default so adding them manually wastes gas

1. File: src/Libraries/LibSwap.sol (line 26)

       uint256 timestamp

2. File: src/Interfaces/ILiFi.sol (line 29)

       uint256 timestamp

3. File: src/Interfaces/ILiFi.sol (line 37)

       uint256 timestamp

4. File: src/Interfaces/ILiFi.sol (line 49)

       uint256 timestamp

5. File: src/Interfaces/ILiFi.sol (line 60)

       uint256 timestamp

++i/i++ should be unchecked{++i}/unchecked{++i} when it is not possible for them to overflow, as is the case when used in for- and while-loops

1. File: src/Libraries/LibDiamond.sol (line 67)

       for (uint256 facetIndex; facetIndex < _diamondCut.length; facetIndex++) {

2. File: src/Libraries/LibDiamond.sol (line 92)

       for (uint256 selectorIndex; selectorIndex < _functionSelectors.length; selectorIndex++) {

3. File: src/Libraries/LibDiamond.sol (line 110)

       for (uint256 selectorIndex; selectorIndex < _functionSelectors.length; selectorIndex++) {

4. File: src/Libraries/LibDiamond.sol (line 125)

       for (uint256 selectorIndex; selectorIndex < _functionSelectors.length; selectorIndex++) {

5. File: src/Facets/DexManagerFacet.sol (line 33)

       for (uint256 i; i < _dexs.length; i++) {

6. File: src/Facets/DexManagerFacet.sol (line 52)

       for (uint256 i; i < s.dexs.length; i++) {

7. File: src/Facets/DexManagerFacet.sol (line 65)

       for (uint256 i; i < _dexs.length; i++) {

8. File: src/Facets/DexManagerFacet.sol (line 70)

           for (uint256 j; j < s.dexs.length; j++) {

9. File: src/Facets/HopFacet.sol (line 48)

       for (uint8 i; i < _tokens.length; i++) {

10. File: src/Facets/Swapper.sol (line 14)

        for (uint8 i; i < _swapData.length; i++) {

11. File: src/Facets/DiamondLoupeFacet.sol (line 24)

        for (uint256 i; i < numFacets; i++) {

<array>.length should not be looked up in every loop of a for-loop

Even memory arrays incur the overhead of bit tests and bit shifts to calculate the array length

1. File: src/Libraries/LibDiamond.sol (line 67)

       for (uint256 facetIndex; facetIndex < _diamondCut.length; facetIndex++) {

2. File: src/Libraries/LibDiamond.sol (line 92)

       for (uint256 selectorIndex; selectorIndex < _functionSelectors.length; selectorIndex++) {

3. File: src/Libraries/LibDiamond.sol (line 110)

       for (uint256 selectorIndex; selectorIndex < _functionSelectors.length; selectorIndex++) {

4. File: src/Libraries/LibDiamond.sol (line 125)

       for (uint256 selectorIndex; selectorIndex < _functionSelectors.length; selectorIndex++) {

5. File: src/Facets/DexManagerFacet.sol (line 33)

       for (uint256 i; i < _dexs.length; i++) {

6. File: src/Facets/DexManagerFacet.sol (line 52)

       for (uint256 i; i < s.dexs.length; i++) {

7. File: src/Facets/DexManagerFacet.sol (line 65)

       for (uint256 i; i < _dexs.length; i++) {

8. File: src/Facets/DexManagerFacet.sol (line 70)

           for (uint256 j; j < s.dexs.length; j++) {

9. File: src/Facets/HopFacet.sol (line 48)

       for (uint8 i; i < _tokens.length; i++) {

10. File: src/Facets/Swapper.sol (line 14)

        for (uint8 i; i < _swapData.length; i++) {

Using calldata instead of memory for read-only arguments in external functions saves gas

1. File: src/Facets/HopFacet.sol (line 41)

       string[] memory _tokens,

2. File: src/Facets/HopFacet.sol (line 42)

       IHopBridge.BridgeConfig[] memory _bridgeConfigs,

require() or revert() statements that check input arguments should be at the top of the function

1. File: src/Libraries/LibDiamond.sol (line 86)

       require(_facetAddress != address(0), "LibDiamondCut: Add facet can't be address(0)");

2. File: src/Libraries/LibDiamond.sol (line 104)

       require(_facetAddress != address(0), "LibDiamondCut: Add facet can't be address(0)");

3. File: src/Libraries/LibDiamond.sol (line 124)

       require(_facetAddress == address(0), "LibDiamondCut: Remove facet address must be address(0)");

internal functions only called once can be inlined to save gas

1. File: src/Facets/CBridgeFacet.sol (line 176)

   function _bridge() internal view returns (address) {

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

1. File: src/Libraries/LibDiamond.sol (line 67)

       for (uint256 facetIndex; facetIndex < _diamondCut.length; facetIndex++) {

2. File: src/Libraries/LibDiamond.sol (line 92)

       for (uint256 selectorIndex; selectorIndex < _functionSelectors.length; selectorIndex++) {

3. File: src/Libraries/LibDiamond.sol (line 110)

       for (uint256 selectorIndex; selectorIndex < _functionSelectors.length; selectorIndex++) {

4. File: src/Libraries/LibDiamond.sol (line 125)

       for (uint256 selectorIndex; selectorIndex < _functionSelectors.length; selectorIndex++) {

5. File: src/Facets/DexManagerFacet.sol (line 33)

       for (uint256 i; i < _dexs.length; i++) {

6. File: src/Facets/DexManagerFacet.sol (line 52)

       for (uint256 i; i < s.dexs.length; i++) {

7. File: src/Facets/DexManagerFacet.sol (line 65)

       for (uint256 i; i < _dexs.length; i++) {

8. File: src/Facets/DexManagerFacet.sol (line 70)

           for (uint256 j; j < s.dexs.length; j++) {

9. File: src/Facets/HopFacet.sol (line 48)

       for (uint8 i; i < _tokens.length; i++) {

10. File: src/Facets/Swapper.sol (line 14)

        for (uint8 i; i < _swapData.length; i++) {

11. File: src/Facets/DiamondLoupeFacet.sol (line 24)

        for (uint256 i; i < numFacets; i++) {

Use a more recent version of solidity

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

1. File: src/Libraries/LibSwap.sol (line 2)

   pragma solidity ^0.8.7;

2. File: src/Libraries/LibUtil.sol (line 2)

   pragma solidity ^0.8.7;

3. File: src/Libraries/LibDiamond.sol (line 2)

   pragma solidity ^0.8.7;

4. File: src/Libraries/LibAsset.sol (line 2)

   pragma solidity ^0.8.7;

5. File: src/Libraries/LibStorage.sol (line 2)

   pragma solidity ^0.8.7;

6. File: src/Interfaces/IAnyswapRouter.sol (line 2)

   pragma solidity ^0.8.7;

7. File: src/Interfaces/IERC165.sol (line 2)

   pragma solidity ^0.8.7;

8. File: src/Interfaces/IHopBridge.sol (line 2)

   pragma solidity ^0.8.7;

9. File: src/Interfaces/IDiamondCut.sol (line 2)

   pragma solidity ^0.8.7;

10. File: src/Interfaces/IDiamondLoupe.sol (line 2)

    pragma solidity ^0.8.7;

11. File: src/Interfaces/ITransactionManager.sol (line 2)

    pragma solidity 0.8.7;

12. File: src/Interfaces/IERC173.sol (line 2)

    pragma solidity ^0.8.7;

13. File: src/Interfaces/IAnyswapToken.sol (line 2)

    pragma solidity ^0.8.7;

14. File: src/Interfaces/ILiFi.sol (line 2)

    pragma solidity ^0.8.7;

15. File: src/Interfaces/ICBridge.sol (line 2)

    pragma solidity ^0.8.7;

16. File: src/LiFiDiamond.sol (line 2)

    pragma solidity ^0.8.7;

17. File: src/Facets/DexManagerFacet.sol (line 2)

    pragma solidity ^0.8.7;

18. File: src/Facets/NXTPFacet.sol (line 2)

    pragma solidity ^0.8.7;

19. File: src/Facets/OwnershipFacet.sol (line 2)

    pragma solidity ^0.8.7;

20. File: src/Facets/HopFacet.sol (line 2)

    pragma solidity ^0.8.7;

21. File: src/Facets/Swapper.sol (line 2)

    pragma solidity ^0.8.7;

22. File: src/Facets/GenericSwapFacet.sol (line 2)

    pragma solidity ^0.8.7;

23. File: src/Facets/DiamondCutFacet.sol (line 2)

    pragma solidity ^0.8.7;

24. File: src/Facets/WithdrawFacet.sol (line 2)

    pragma solidity ^0.8.7;

25. File: src/Facets/AnyswapFacet.sol (line 2)

    pragma solidity ^0.8.7;

26. File: src/Facets/DiamondLoupeFacet.sol (line 2)

    pragma solidity ^0.8.7;

27. File: src/Facets/CBridgeFacet.sol (line 2)

    pragma solidity ^0.8.7;

Splitting require() statements that use && saves gas

See this issue for an example

1. File: src/Facets/Swapper.sol (lines 15-18)

           require(
               ls.dexWhitelist[_swapData[i].approveTo] == true && ls.dexWhitelist[_swapData[i].callTo] == true,
               "Contract call not allowed!"
           );

Duplicated require()/revert() checks should be refactored to a modifier or function

1. File: src/Libraries/LibDiamond.sol (line 102)

       require(_functionSelectors.length > 0, "LibDiamondCut: No selectors in facet to cut");

2. File: src/Libraries/LibDiamond.sol (line 104)

       require(_facetAddress != address(0), "LibDiamondCut: Add facet can't be address(0)");

3. File: src/Facets/AnyswapFacet.sol (line 105)

           require(_postSwapBalance > 0, "ERR_INVALID_AMOUNT");

4. File: src/Facets/CBridgeFacet.sol (line 116)

           require(_postSwapBalance > 0, "ERR_INVALID_AMOUNT");

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

1. File: src/Libraries/LibDiamond.sol (line 212)

       require(contractSize > 0, _errorMessage);

2. File: src/Facets/NXTPFacet.sol (line 98)

       require(_postSwapBalance > 0, "ERR_INVALID_AMOUNT");

3. File: src/Facets/HopFacet.sol (line 109)

       require(_postSwapBalance > 0, "ERR_INVALID_AMOUNT");

4. File: src/Facets/AnyswapFacet.sol (line 92)

           require(_postSwapBalance > 0, "ERR_INVALID_AMOUNT");

5. File: src/Facets/AnyswapFacet.sol (line 105)

           require(_postSwapBalance > 0, "ERR_INVALID_AMOUNT");

6. File: src/Facets/CBridgeFacet.sol (line 105)

           require(_postSwapBalance > 0, "ERR_INVALID_AMOUNT");

7. File: src/Facets/CBridgeFacet.sol (line 116)

           require(_postSwapBalance > 0, "ERR_INVALID_AMOUNT");

require()/revert() strings longer than 32 bytes cost extra gas

1. File: src/Libraries/LibDiamond.sol (line 56)

       require(msg.sender == diamondStorage().contractOwner, "LibDiamond: Must be contract owner");

2. File: src/Libraries/LibDiamond.sol (line 76)

               revert("LibDiamondCut: Incorrect FacetCutAction");

3. File: src/Libraries/LibDiamond.sol (line 84)

       require(_functionSelectors.length > 0, "LibDiamondCut: No selectors in facet to cut");

4. File: src/Libraries/LibDiamond.sol (line 86)

       require(_facetAddress != address(0), "LibDiamondCut: Add facet can't be address(0)");

5. File: src/Libraries/LibDiamond.sol (line 95)

           require(oldFacetAddress == address(0), "LibDiamondCut: Can't add function that already exists");

6. File: src/Libraries/LibDiamond.sol (line 102)

       require(_functionSelectors.length > 0, "LibDiamondCut: No selectors in facet to cut");

7. File: src/Libraries/LibDiamond.sol (line 104)

       require(_facetAddress != address(0), "LibDiamondCut: Add facet can't be address(0)");

8. File: src/Libraries/LibDiamond.sol (line 113)

           require(oldFacetAddress != _facetAddress, "LibDiamondCut: Can't replace function with same function");

9. File: src/Libraries/LibDiamond.sol (line 121)

       require(_functionSelectors.length > 0, "LibDiamondCut: No selectors in facet to cut");

10. File: src/Libraries/LibDiamond.sol (line 124)

        require(_facetAddress == address(0), "LibDiamondCut: Remove facet address must be address(0)");

11. File: src/Libraries/LibDiamond.sol (line 154)

        require(_facetAddress != address(0), "LibDiamondCut: Can't remove function that doesn't exist");

12. File: src/Libraries/LibDiamond.sol (line 156)

        require(_facetAddress != address(this), "LibDiamondCut: Can't remove immutable function");

13. File: src/Libraries/LibDiamond.sol (line 187)

            require(_calldata.length == 0, "LibDiamondCut: _init is address(0) but_calldata is not empty");

14. File: src/Libraries/LibDiamond.sol (line 189)

            require(_calldata.length > 0, "LibDiamondCut: _calldata is empty but _init is not address(0)");

15. File: src/Libraries/LibDiamond.sol (line 200)

                    revert("LibDiamondCut: _init function reverted");

16. File: src/Facets/HopFacet.sol (line 146)

        require(s.hopChainId != _hopData.chainId, "Cannot bridge to the same network.");

17. File: src/Facets/AnyswapFacet.sol (line 133)

        require(block.chainid != _anyswapData.toChainId, "Cannot bridge to the same network.");

18. File: src/Facets/CBridgeFacet.sol (line 147)

        require(s.cBridgeChainId != _cBridgeData.dstChainId, "Cannot bridge to the same network.");

State variables should be cached in stack variables rather than re-reading them from storage

The instances below point to the second access of a state variable within a function. Less obvious optimizations include having local storage variables of mappings within state variable mappings or mappings within state variable structs, having local storage variables of structs within mappings, or having local caches of state variable contracts/addresses.

1. File: src/Facets/DexManagerFacet.sol (line 24)

       s.dexWhitelist[_dex] = true;

2. File: src/Facets/DexManagerFacet.sol (line 37)

           s.dexWhitelist[_dexs[i]] = true;

3. File: src/Facets/DexManagerFacet.sol (line 51)

       s.dexWhitelist[_dex] = false;

4. File: src/Facets/DexManagerFacet.sol (line 69)

           s.dexWhitelist[_dexs[i]] = false;

5. File: src/Facets/DexManagerFacet.sol (line 85)

       s.dexs[index] = s.dexs[s.dexs.length - 1];

6. File: src/Facets/Swapper.sol (line 16)

               ls.dexWhitelist[_swapData[i].approveTo] == true && ls.dexWhitelist[_swapData[i].callTo] == true,

Usage of uints/ints smaller than 32 bytes (256 bits) incurs overhead

When using elements that are smaller than 32 bytes, your contract’s gas usage may be higher. This is because the EVM operates on 32 bytes at a time. Therefore, if the element is smaller than that, the EVM must use more operations in order to reduce the size of the element from 32 bytes to the desired size.

https://docs.soliditylang.org/en/v0.8.11/internals/layout_in_storage.html Use a larger size then downcast where needed

1. File: src/Libraries/LibDiamond.sol (line 11)

       uint96 functionSelectorPosition; // position in facetFunctionSelectors.functionSelectors array

2. File: src/Libraries/LibDiamond.sol (line 87)

       uint96 selectorPosition = uint96(ds.facetFunctionSelectors[_facetAddress].functionSelectors.length);

3. File: src/Libraries/LibDiamond.sol (line 105)

       uint96 selectorPosition = uint96(ds.facetFunctionSelectors[_facetAddress].functionSelectors.length);

4. File: src/Libraries/LibDiamond.sol (line 141)

       uint96 _selectorPosition,

5. File: src/Interfaces/ILiFi.sol (line 62)

   event Inited(address indexed bridge, uint64 chainId);

6. File: src/Interfaces/ICBridge.sol (line 9)

       uint64 _dstChinId,

7. File: src/Interfaces/ICBridge.sol (line 10)

       uint64 _nonce,

8. File: src/Interfaces/ICBridge.sol (line 11)

       uint32 _maxSlippage

9. File: src/Interfaces/ICBridge.sol (line 17)

       uint64 _dstChinId,

10. File: src/Interfaces/ICBridge.sol (line 18)

        uint64 _nonce,

11. File: src/Interfaces/ICBridge.sol (line 19)

        uint32 _maxSlippage

12. File: src/Facets/HopFacet.sol (line 48)

        for (uint8 i; i < _tokens.length; i++) {

13. File: src/Facets/Swapper.sol (line 14)

        for (uint8 i; i < _swapData.length; i++) {

14. File: src/Facets/CBridgeFacet.sol (line 21)

        uint64 cBridgeChainId;

15. File: src/Facets/CBridgeFacet.sol (line 30)

        uint64 dstChainId;

16. File: src/Facets/CBridgeFacet.sol (line 31)

        uint64 nonce;

17. File: src/Facets/CBridgeFacet.sol (line 32)

        uint32 maxSlippage;

18. File: src/Facets/CBridgeFacet.sol (line 42)

    function initCbridge(address _cBridge, uint64 _chainId) external {

Structs can be packed into fewer storage slots

1. File: src/Facets/CBridgeFacet.sol (lines 26-33)

   struct CBridgeData {
       address receiver;
       address token;
       uint256 amount;
       uint64 dstChainId;
       uint64 nonce;
       uint32 maxSlippage;
   }

   Struct member ordering with 3 slots instead of the current 4: uint256(32):amount,address(20):receiver,uint64(8):dstChainId,uint32(4):maxSlippage,address(20):token,uint64(8):nonce

Remove unused variables

1. File: src/Libraries/LibSwap.sol (line 8)

   uint256 private constant MAX_INT = 2**256 - 1;

Use custom error codes instead of revert strings to save gas

Various files and various locations

[H3xept]

Re: Expressions for constant values such as a call to keccak256(), should use immutable rather than constant

Fixed by lifinance/lifi-contracts@39dd074acf47b40f9a544439427e58de0208b961

[H3xept]

Re: ++i/i++ should be unchecked{++i}/unchecked{++i} when it is not possible for them to overflow, as is the case when used in for- and while-loops

We internally decided to avoid unchecked expressions for now.

[H3xept]

Re: Splitting require() statements that use && saves gas -- I found no evidence online about this. Upon testing locally, this seems to be wrong (&& saves gas!!)

[H3xept]

Re: .length should not be looked up in every loop of a for-loop

Fixed by lifinance/lifi-contracts@975f12529f2232a59def392349bf8dccf4141aa9 Duplicate of #44

[H3xept]

Re: require()/revert() strings longer than 32 bytes cost extra gas

Fixed by lifinance/lifi-contracts@45edddfb56028db3cfd070b57990ae8a455f0109

[H3xept]

Re: State variables should be cached in stack variables rather than re-reading them from storage

Fixed by lifinance/lifi-contracts@2b0c057fb05c62d95c0b04edd1864c184ccf9ad8

[H3xept]

Re Unused variable MAX_INT

Duplicate of #100

[H3xept]

Re strings longer than 32 bytes cost extra gas

Duplicate of #100

[H3xept]

Re Structs can be packed into fewer storage slots

Duplicate of #178

[H3xept]

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

Duplicate of #100

[H3xept]

Re prefix increments

We internally decided to avoid previx increments for now.

[H3xept]

Re calldata instead of memory for read-only arguments

Duplicate of #152

[H3xept]

Re pre-compute constant values

Duplicate of #182