Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Gas Optimizations #167

Open
code423n4 opened this issue Mar 30, 2022 · 6 comments
Open

Gas Optimizations #167

code423n4 opened this issue Mar 30, 2022 · 6 comments
Labels
bug Something isn't working G (Gas Optimization) resolved Finding has been patched by sponsor (sponsor pls link to PR containing fix) sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons

Comments

@code423n4
Copy link
Contributor

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

@code423n4 code423n4 added bug Something isn't working G (Gas Optimization) labels Mar 30, 2022
code423n4 added a commit that referenced this issue Mar 30, 2022
@H3xept
Copy link
Collaborator

H3xept commented Apr 8, 2022

Re Function can set external

Duplicate of #197

@H3xept
Copy link
Collaborator

H3xept commented Apr 8, 2022

Re Redundant literal boolean comparisons

Duplicate of #39

@H3xept
Copy link
Collaborator

H3xept commented Apr 11, 2022

Re != more effective than <

Duplicate of #100

@H3xept
Copy link
Collaborator

H3xept commented Apr 11, 2022

Re unchecked operations

We internally decided to avoid unchecked operations for now.

@H3xept H3xept added the sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons label Apr 11, 2022
@H3xept
Copy link
Collaborator

H3xept commented Apr 11, 2022

Re prefix increments

We internally decided to avoid previx increments for now.

@H3xept
Copy link
Collaborator

H3xept commented Apr 11, 2022

Re calldata instead of memory for read-only arguments

Duplicate of #152

@H3xept H3xept closed this as completed Apr 11, 2022
@H3xept H3xept added the resolved Finding has been patched by sponsor (sponsor pls link to PR containing fix) label Apr 11, 2022
@H3xept H3xept reopened this Apr 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working G (Gas Optimization) resolved Finding has been patched by sponsor (sponsor pls link to PR containing fix) sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons
Projects
None yet
Development

No branches or pull requests

2 participants