code-423n4 / 2022-03-lifinance-findings

6 stars 4 forks source link

Gas Optimizations #51

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Gas Optimizations

constant expressions

expression assigned to constants are recomputed every time it is called, so keccak operation is done every time the variable is used This can be prevented by using value directly or using immutable so that value is computed once

Proof of concept

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/CBridgeFacet.sol#L18

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/NXTPFacet.sol#L18

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/HopFacet.sol#L18

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Libraries/LibDiamond.sol#L7

Mitigation

Use immutable instead of constant

using unchecked can save gas

arithmetic operations that cant overflow/underflow can placed inside a unchecked block to avoid underflow/overflow check and save gas

Proof of concept

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/NXTPFacet.sol#L166

Mitigation

statement can be placed inside a unchecked block

Storage variable can be cached and re-used to save gas

Storing storage variable re-using it can save ~100 gas on repeated reads

Proof of concept

In DexManagerFacet:removeDex s.dex.length is read in every iteration, this value can be cached in a variable and save ~100 gas in repeated storage reads

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/DexManagerFacet.sol#L52

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/DexManagerFacet.sol#L70

In HopFacet:_startBridge s.hopChainId can be cached

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/HopFacet.sol#L139-L153

Replacing > with != can save gas

using != can save gas than > with uint and optimizer enabled

Proof of concept

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/AnyswapFacet.sol#L92

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/CBridgeFacet.sol#L105

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/NXTPFacet.sol#L98

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/DexManagerFacet.sol#L70

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/HopFacet.sol#L109

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Libraries/LibDiamond.sol#L84

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Libraries/LibDiamond.sol#L102

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Libraries/LibDiamond.sol#L121

Repeating statements

In AnyswapFacet:swapAndStartBridgeTokensViaAnyswap, require and update statements can be placed outside if-else statements

    require(_postSwapBalance > 0, "ERR_INVALID_AMOUNT");
    _anyswapData.amount = _postSwapBalance;

Proof of concept

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/AnyswapFacet.sol#L92-L94

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/CBridgeFacet.sol#L105-L107

Mitigation

the two statements can be placed outide the if-else block

Revert strings can <= 32 bytes

reducing revert string to under 32 bytes can save deployment costs as well as when require condition fails

https://blog.polymath.network/solidity-tips-and-tricks-to-save-gas-and-reduce-bytecode-size-c44580b218e6#c17b

Proof of concept

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/AnyswapFacet.sol#L133

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/CBridgeFacet.sol#L147

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/HopFacet.sol#L146

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Libraries/LibDiamond.sol#L76

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Libraries/LibDiamond.sol#L84

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Libraries/LibDiamond.sol#L133

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Libraries/LibDiamond.sol#L154

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Libraries/LibDiamond.sol#L189

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Libraries/LibDiamond.sol#L200

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/LiFiDiamond.sol#L38

Mitigation

Pre-increment is more efficient than post increment

prefix increment uses less gas when compared to postfix increment unchecked can be added to save more gas as the variable wont overflow

https://github.com/ethereum/solidity/issues/10695

Proof of concept

In all for loop in the project

Examples https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/DexManagerFacet.sol#L33

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/DexManagerFacet.sol#L52

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/DiamondLoupeFacet.sol#L24

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/HopFacet.sol#L48

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/Swapper.sol#L14

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Libraries/LibDiamond.sol#L67

In _executeSwaps() LifiData can be replaced with bytes32

In function _executeSwaps() input LifiData only transaction ID is used from the structure lifiData , so it can be replaced with a bytes32 lifiData.transactionID

Proof of concept

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/Swapper.sol#L12

Statements can be reordered to save gas on revert

In LibDiamond:removeFunctions second require statement can be placed above storage read to save gas on revert

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

    DiamondStorage storage ds = diamondStorage();

    // if function does not exist then do nothing and return
    require(_facetAddress == address(0), "LibDiamondCut: Remove facet address must be address(0)");

Proof of concept

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Libraries/LibDiamond.sol#L84-L86

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Libraries/LibDiamond.sol#L102-L104

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Libraries/LibDiamond.sol#L124

Mitigation

Place require statement above storage read

if statements can be nested

In LibSwap:swap if statement can be combined to reduce one call

    if (!LibAsset.isNativeAsset(fromAssetId) && LibAsset.getOwnBalance(fromAssetId) < fromAmount) {
        LibAsset.transferFromERC20(fromAssetId, msg.sender, address(this), fromAmount);
    }

    if (!LibAsset.isNativeAsset(fromAssetId)) {
        LibAsset.approveERC20(IERC20(fromAssetId), _swapData.approveTo, fromAmount);
    }

Proof of concept

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Libraries/LibSwap.sol#L33

Mitigation

if blocks can be nested


if (!LibAsset.isNativeAsset(fromAssetId)) { 

    if(LibAsset.getOwnBalance(fromAssetId) < fromAmount) {
        LibAsset.transferFromERC20(fromAssetId, msg.sender, address(this), fromAmount);
    }

    LibAsset.approveERC20(IERC20(fromAssetId), _swapData.approveTo, fromAmount);
}
H3xept commented 2 years ago

Re unchecked & prefix increment

We internally decided to avoid unchecked statements and prefix increments for now.

H3xept commented 2 years ago
  1. Constant expressions: Fixed in lifinfance/lifi-contracts/@39dd074acf47b40f9a544439427e58de0208b961
  2. Storage variable caching: Fixed in lifinance/lifi-contracts/@2b0c057fb05c62d95c0b04edd1864c184ccf9ad8
  3. 32 bytes revert string: Fixed in lifinance/lifi-contracts/@45edddfb56028db3cfd070b57990ae8a455f0109
  4. In _executeSwaps() LifiData can be replaced with bytes32: To maintain the same interface in future updates we should leave this as it is
H3xept commented 2 years ago

Re nesting Ifs

Duplicate of #39

H3xept commented 2 years ago

Re Revert strings can <= 32 bytes

Duplicate of #100

H3xept commented 2 years ago

Re Storage variable can be cached and re-used to save gas

Duplicate of #196

H3xept commented 2 years ago

Re Replacing > with != can save gas

Duplicate of #100

H3xept commented 2 years ago

Re prefix increments

We internally decided to avoid previx increments for now.

H3xept commented 2 years ago

Re constant pre-computation

Duplicate of #182