search

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

6 stars 4 forks source link

Gas Optimizations #167

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Gas opt

1 Using == true cost more gas

https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/Swapper.sol#L16 Using == true to validate bool variable is unnecessary:

require(ls.dexWhitelist[_swapData[i].approveTo]);

2 Using && in require() can cost more execution gas fee

https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/Swapper.sol#L15-L18 Using multiple require() can save execution gas:

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

3 Using unchecked and prefix increment for loop increment

https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/Swapper.sol#L14 Using unchecked can save gas:

for (uint8 i; i < _swapData.length) {
            require(
                ls.dexWhitelist[_swapData[i].approveTo] == true && ls.dexWhitelist[_swapData[i].callTo] == true,
                "Contract call not allowed!"
            );
Unchecked{++i}

4 Relocate the toAmount var declaration location

https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Libraries/LibSwap.sol#L31 Declare the var at L48, then put the value LibAsset.getOwnBalance(_swapData.receivingAssetId) replacing toAmount. Remove L31

uint toAmount = LibAsset.getOwnBalance(_swapData.receivingAssetId) - LibAsset.getOwnBalance(_swapData.receivingAssetId);

5 Using parameter to emit event

https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/CBridgeFacet.sol#L47 Instead of reading from storage to get s.cBridge and s.cBridgeChainId value, using _cBridge and _chainId to emit can save gas

6 Unnecessary _sendingAssetIdBalance var declaration

https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/NXTPFacet.sol#L54-L57 _sendingAssetIdBalance was only called once in the contract. Caching LibAsset.getOwnBalance(sendingAssetId) into memory is consuming gas. I recommend just put it directly at L57

require(
                LibAsset.getOwnBalance(sendingAssetId) - LibAsset.getOwnBalance(sendingAssetId) ==  _nxtpData.amount,
                "ERR_INVALID_AMOUNT"
            );

Same case at: https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/NXTPFacet.sol#L91

7 Using calldata on struct parameter

https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/Swapper.sol#L12 https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/AnyswapFacet.sol#L35 https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/AnyswapFacet.sol#L75 Using calldata to store struct data type (LifiData) can save gas

8 != more effective than <

https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/AnyswapFacet.sol#L92 https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/AnyswapFacet.sol#L105 Using != to validate than value is not 0 is gas saving

9 Using unchecked on calculating finalBalance

https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/NXTPFacet.sol#L166 The calculation of finalBalance won't underflow because of the if condition at L165. Using unchecked will save gas:

Unchecked{
finalBalance = postSwapBalance - startingBalance;
}

10 Unnecessary receivingAssetIdBalance MSTORE

https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/GenericSwapFacet.sol#L23 receivingAssetIdBalance var was merely called once in the function. Just pass LibAsset.getOwnBalance(_lifiData.receivingAssetId) value directly to L28:

uint256 postSwapBalance = LibAsset.getOwnBalance(_lifiData.receivingAssetId) - LibAsset.getOwnBalance(_lifiData.receivingAssetId);

11 Custom errors from Solidity 0.8.4 are cheaper than revert strings

By defined using error statement, and using if(condition)revert() to check the condition. It can be implemented for all require() check

12 Function can set external

https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/AnyswapFacet.sol#L35 https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/AnyswapFacet.sol#L78 https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/CBridgeFacet.sol#L57 https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/CBridgeFacet.sol#L96 Some function can set external to save gas

H3xept commented 2 years ago

Re Function can set external

Duplicate of #197

H3xept commented 2 years ago

Re Redundant literal boolean comparisons

Duplicate of #39

H3xept commented 2 years ago

Re != more effective than <

Duplicate of #100

H3xept commented 2 years ago

Re unchecked operations

We internally decided to avoid unchecked operations for now.

H3xept commented 2 years ago

Re prefix increments

We internally decided to avoid previx increments for now.

H3xept commented 2 years ago

Re calldata instead of memory for read-only arguments

Duplicate of #152