Open code423n4 opened 2 years ago
Duplicate of #197
Duplicate of #44
Duplicate of #100
Duplicate of #196
Duplicate of #100
Duplicate of #196
We internally decided to avoid unchecked operations for now.
We internally decided to avoid previx increments for now.
1. Prefix increments are cheaper than postfix increments
Impact
There is no risk of overflow caused by increamenting the iteration index in for loops (the i++ in for for (uint8 i; i < _swapData.length; i++) {. Increments perform overflow checks that are not necessary in this case.
Proof of Concept
https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/Swapper.sol#L14
Recommended Mitigation Steps
Surround the increment expressions with an unchecked { ... } block to avoid the default overflow checks.
2.
uint8
is less efficient thanuint256
in loop iterationsImpact
This loop use uint8 for an index parameter (i). It does not give any efficiency, actually, it is the opposite as EVM operates on default of 256-bit values so uint8 is more expensive in this case as it needs a conversion. It only gives improvements in cases where you can pack variables together, e.g. structs.
Proof of Concept
https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/Swapper.sol#L14
Recommended Mitigation Steps
Replace uint8 with uint256 in loop iterations.
3. > 0 can be replaced with != 0 for gas optimisation
Impact
!= 0 is a cheaper operation compared to > 0, when dealing with uint.
Proof of Concept
https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/CBridgeFacet.sol#L116 There are several other places throughout the codebase where the same optimization can be used.
4. Use Custom Errors to save Gas
Impact
Custom errors are cheaper than revert strings.
Proof of Concept
Source: https://blog.soliditylang.org/2021/04/21/custom-errors/: Starting from Solidity v0.8.4, there is a convenient and gas-efficient way to explain to users why an operation failed through the use of custom errors. Until now, you could already use strings to give more information about failures (e.g., revert("Insufficient funds.");), but they are rather expensive, especially when it comes to deploy cost, and it is difficult to use dynamic information in them. Custom errors are defined using the error statement, which can be used inside and outside of contracts (including interfaces and libraries).
Recommended Mitigation Steps
Replace revert strings with custom errors.
5. Long Revert Strings
Impact
Shortening revert strings to fit in 32 bytes will decrease deployment time gas and will decrease runtime gas when the revert condition has been met.
Proof of Concept
https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/CBridgeFacet.sol#L147
There are several other places throughout the codebase where the same optimization can be used.
https://planetcalc.com/9029/
Recommended Mitigation Steps
Shorten the revert strings to fit in 32 bytes
6. Adding unchecked directive can save gas (6 findings)
Impact
For the arithmetic operations that will never over/underflow, using the unchecked directive (Solidity v0.8 has default overflow/underflow checks) can save some gas from the unnecessary internal over/underflow checks.
Proof of Concept
https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/NXTPFacet.sol#L166
Recommended Mitigation Steps
Consider using 'unchecked' where it is safe to do so.
6. && operator can use more gas
Impact
More expensive gas usage
Proof of Concept
https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/Swapper.sol#L16
Recommended Mitigation Steps
Instead of using operator && on single require check using double require check can save more gas
7. Avoid use of state variables in event emissions to save gas
Impact
Where possible, use equivalent function parameters or local variables in event emits instead of state variables to prevent expensive SLOADs. Post-Berlin, SLOADs on state variables accessed first-time in a transaction increased from 800 gas to 2100, which is a 2.5x increase.
Proof of Concept
https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/CBridgeFacet.sol#L47 The
Inited
event i uses state variables.cBridge
ands.cBridgeChainId
instead of using the equivalent function parameters_cBridge
and_chainId
which were just used to set these state variables.Recommended Mitigation Steps
Use equivalent function parameters or local variables in event emits instead of state variables.
8. Free gas savings for using solidity 0.8.10+
Impact
Gas savings
Proof of Concept
Solidity 0.8.10 has a useful change which reduced gas costs of external calls which expect a return value: https://blog.soliditylang.org/2021/11/09/solidity-0.8.10-release-announcement/
LIFI protocol is using 0.8.6: Updating to the newer version of solc will allow contracts to take advantage of these lower costs for external calls.
Recommended Mitigation Steps
Update to 0.8.10+
9. Caching variables (3 findings)
Impact
Some of the variable can be cached to slightly reduce gas usage.
Proof of Concept
LibAsset
can be cached. https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/NXTPFacet.sol#L46-L76https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/NXTPFacet.sol#L124-L140
IERC20(_assetAddress)
can be cached. https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/WithdrawFacet.sol#L33-L35Recommended Mitigation Steps
Consider caching those variable for read and make sure write back to storage
10. Gas optimization on 2**256 - 1
Vulnerability details
Impact
when using 2**256 - 1 it takes more gas, than using type(uint256).max
Proof of Concept
https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Libraries/LibAsset.sol#L15 https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Libraries/LibSwap.sol#L8
Recommended Mitigation Steps
Use type(uint256).max