-
Notifications
You must be signed in to change notification settings - Fork 4
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 #196
Comments
Re: Expressions for constant values such as a call to keccak256(), should use immutable rather than constantFixed by lifinance/lifi-contracts@39dd074acf47b40f9a544439427e58de0208b961 |
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-loopsWe internally decided to avoid unchecked expressions for now. |
Re: Splitting require() statements that use && saves gasI found no evidence online about this. Upon testing locally, this seems to be wrong (&& saves gas!!) |
Re: .length should not be looked up in every loop of a for-loopFixed by lifinance/lifi-contracts@975f12529f2232a59def392349bf8dccf4141aa9 |
Re: require()/revert() strings longer than 32 bytes cost extra gasFixed by lifinance/lifi-contracts@45edddfb56028db3cfd070b57990ae8a455f0109 |
Re: State variables should be cached in stack variables rather than re-reading them from storageFixed by lifinance/lifi-contracts@2b0c057fb05c62d95c0b04edd1864c184ccf9ad8 |
Re Unused variable MAX_INTDuplicate of #100 |
Re strings longer than 32 bytes cost extra gasDuplicate of #100 |
Re Structs can be packed into fewer storage slotsDuplicate of #178 |
Re Using > 0 costs more gas than != 0 when used on a uint in a require() statementDuplicate of #100 |
Re prefix incrementsWe internally decided to avoid previx increments for now. |
Re calldata instead of memory for read-only argumentsDuplicate of #152 |
Re pre-compute constant valuesDuplicate of #182 |
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>)
Expressions for constant values such as a call to
keccak256()
, should useimmutable
rather thanconstant
See this issue for a detail description of the issue
Superfluous event fields
block.timestamp
andblock.number
are added to event information by default so adding them manually wastes gas++i
/i++
should beunchecked{++i}
/unchecked{++i}
when it is not possible for them to overflow, as is the case when used infor
- andwhile
-loops<array>.length
should not be looked up in every loop of afor
-loopEven memory arrays incur the overhead of bit tests and bit shifts to calculate the array length
Using
calldata
instead ofmemory
for read-only arguments inexternal
functions saves gasrequire()
orrevert()
statements that check input arguments should be at the top of the functioninternal
functions only called once can be inlined to save gas++i
costs less gas than++i
, especially when it's used infor
-loops (--i
/i--
too)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
Splitting
require()
statements that use&&
saves gasSee this issue for an example
Duplicated
require()
/revert()
checks should be refactored to a modifier or functionUsing
> 0
costs more gas than!= 0
when used on auint
in arequire()
statementrequire()
/revert()
strings longer than 32 bytes cost extra gasState 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.
Usage of
uints
/ints
smaller than 32 bytes (256 bits) incurs overheadhttps://docs.soliditylang.org/en/v0.8.11/internals/layout_in_storage.html
Use a larger size then downcast where needed
uint64 cBridgeChainId;
uint64 dstChainId;
uint64 nonce;
uint32 maxSlippage;
Structs can be packed into fewer storage slots
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
Use custom error codes instead of revert strings to save gas
Various files and various locations
The text was updated successfully, but these errors were encountered: