Gas Optimizations #223
Labels
bug
Something isn't working
G (Gas Optimization)
G-37
grade-a
high quality report
This report is of especially high quality
selected for report
This submission will be included/highlighted in the audit report
Comments
|
kirk-baird marked the issue as grade-a |
|
kirk-baird marked the issue as selected for report |
c4-judge
added
the
selected for report
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
FINDINGS
NB: Some functions have been truncated where neccessary to just show affected parts of the code.
Throught the report some places might be denoted with audit tags to show the actual place affected.
Using immutable on variables that are only set in the constructor and never after
Use immutable if you want to assign a permanent value at construction. Use constants if you already know the permanent value. Both get directly embedded in bytecode, saving SLOAD.
Variables only set in the constructor and never edited afterwards should be marked as immutable, as it would avoid the expensive storage-writing operation in the constructor (around 20 000 gas per variable) and replace the expensive storage-reading operations (around 2100 gas per reading) to a less expensive value reading (3 gas)
https://github.com/code-423n4/2022-10-paladin/blob/d6d0c0e57ad80f15e9691086c9c7270d4ccfe0e6/contracts/WardenPledge.sol#L60
https://github.com/code-423n4/2022-10-paladin/blob/d6d0c0e57ad80f15e9691086c9c7270d4ccfe0e6/contracts/WardenPledge.sol#L62
Use constants for variables whose value is known beforehand and is never changed
https://github.com/code-423n4/2022-10-paladin/blob/d6d0c0e57ad80f15e9691086c9c7270d4ccfe0e6/contracts/WardenPledge.sol#L79
Cache storage values in memory to minimize SLOADs
The code can be optimizhttps://github.com/code-423n4/2022-08-fiatdao-findings/blob/main/data/GalloDaSballo-G.mded by minimizing the number of SLOADs.
SLOADs are expensive (100 gas after the 1st one) compared to MLOADs/MSTOREs (3 gas each). Storage values read multiple times should instead be cached in memory the first time (costing 1 SLOAD) and then read from this cache to avoid multiple SLOADs.
NB: *Some functions have been truncated where neccessary to just show affected parts of the code
https://github.com/code-423n4/2022-10-paladin/blob/d6d0c0e57ad80f15e9691086c9c7270d4ccfe0e6/contracts/WardenPledge.sol#L222-L274
WardenPledge.sol._pledge(): delegationBoost should be cached(Saves 4 SLOADs ~394 gas)
https://github.com/code-423n4/2022-10-paladin/blob/d6d0c0e57ad80f15e9691086c9c7270d4ccfe0e6/contracts/WardenPledge.sol#L222-L274
WardenPledge.sol._pledge(): pledgeAvailableRewardAmounts[pledgeId] should be cached(saves 1 SLOAD ~97 gas)
https://github.com/code-423n4/2022-10-paladin/blob/d6d0c0e57ad80f15e9691086c9c7270d4ccfe0e6/contracts/WardenPledge.sol#L299-L358
WardenPledge.sol.createPledge(): minAmountRewardToken[rewardToken] should be cached(saves 1 SLOAD ~97 gas) - happy path
require() or revert() statements that check input arguments should be at the top of the function
Checks that involve constants should come before checks that involve state variables, function calls, and calculations. By doing these checks first, the function is able to revert before wasting a Gcoldsload (2100 gas) in a function that may ultimately revert in the unhappy case.
https://github.com/code-423n4/2022-10-paladin/blob/d6d0c0e57ad80f15e9691086c9c7270d4ccfe0e6/contracts/WardenPledge.sol#L222-L274
https://github.com/code-423n4/2022-10-paladin/blob/d6d0c0e57ad80f15e9691086c9c7270d4ccfe0e6/contracts/WardenPledge.sol#L456-L480
https://github.com/code-423n4/2022-10-paladin/blob/d6d0c0e57ad80f15e9691086c9c7270d4ccfe0e6/contracts/WardenPledge.sol#L488-L515
https://github.com/code-423n4/2022-10-paladin/blob/d6d0c0e57ad80f15e9691086c9c7270d4ccfe0e6/contracts/WardenPledge.sol#L525-L533
Using storage instead of memory for structs/arrays saves gas
When fetching data from a storage location, assigning the data to a memory variable causes all fields of the struct/array to be read from storage, which incurs a Gcoldsload (2100 gas) for each field of the struct/array. If the fields are read from the new memory variable, they incur an additional MLOAD rather than a cheap stack read. Instead of declearing the variable with the memory keyword, declaring the variable with the storage keyword and caching any fields that need to be re-read in stack variables, will be much cheaper, only incuring the Gcoldsload for the fields actually read. The only time it makes sense to read the whole struct/array into a memory variable, is if the full struct/array is being returned by the function, is being passed to a function that requires memory, or if the array/struct is being read from another memory array/struct
https://github.com/code-423n4/2022-10-paladin/blob/d6d0c0e57ad80f15e9691086c9c7270d4ccfe0e6/contracts/WardenPledge.sol#L227
Using unchecked blocks to save gas
Solidity version 0.8+ comes with implicit overflow and underflow checks on unsigned integers. When an overflow or an underflow isn’t possible (as an example, when a comparison is made before the arithmetic operation), some gas can be saved by using an unchecked block
see resource
https://github.com/code-423n4/2022-10-paladin/blob/d6d0c0e57ad80f15e9691086c9c7270d4ccfe0e6/contracts/WardenPledge.sol#L268
The operation
pledgeAvailableRewardAmounts[pledgeId] -= rewardAmount;cannot underflow due to the check on Line 267 that ensures thatpledgeAvailableRewardAmounts[pledgeId]is greater thanrewardAmountbefore perfoming the arithmetic operationThe text was updated successfully, but these errors were encountered: