Platform: Code4rena
Start Date: 28/01/2022
Pot Size: $30,000 USDC
Total HM: 4
Participants: 22
Period: 3 days
Judge: GalloDaSballo
Total Solo HM: 2
Id: 80
League: ETH
Rank: 8/22
Findings: 2
Award: $213.59
🌟 Selected for report: 2
🚀 Solo Findings: 0
7.2132 USDC - $7.21
robee
Prefix increments are cheaper than postfix increments.
Further more, using unchecked {++x} is even more gas efficient, and the gas saving accumulates every iteration and can make a real change
There is no risk of overflow caused by increamenting the iteration index in for loops (the ++i
in for (uint256 i = 0; i < numIterations; ++i)
).
But increments perform overflow checks that are not necessary in this case.
These functions use not using prefix increments (++x
) or not using the unchecked keyword:
change to prefix increment and unchecked: ConvexStakingWrapper.sol, i, 115 change to prefix increment and unchecked: ConvexStakingWrapper.sol, u, 172 change to prefix increment and unchecked: ConvexStakingWrapper.sol, u, 227 change to prefix increment and unchecked: ConvexYieldWrapper.sol, i, 111 change to prefix increment and unchecked: ConvexStakingWrapper.sol, i, 287 change to prefix increment and unchecked: ConvexStakingWrapper.sol, i, 271 change to prefix increment and unchecked: ConvexStakingWrapper.sol, i, 315 change to prefix increment and unchecked: ConvexYieldWrapper.sol, i, 63
#0 - devtooligan
2022-02-01T01:39:40Z
neat
42.7547 USDC - $42.75
robee
ou can change the order of the storage variables to decrease memory uses.
In ConvexStakingWrapper.sol,rearranging the storage fields can optimize to: 10 slots from: 11 slots. The new order of types (you choose the actual variables): 1. uint256 2. uint256 3. uint256 4. address 5. bool 6. bool 7. bool 8. bool 9. address 10. address 11. address 12. address 13. address 14. address
#0 - devtooligan
2022-02-01T02:00:27Z
I'm not clear on the suggestion here. Are you suggesting re-defining the structs? Can you provide a specific example of what the rearranging will look like?
#1 - devtooligan
2022-02-02T16:12:44Z
Going with the better explanation and presented solution #58
25.6528 USDC - $25.65
robee
Unnecessary default assignments, you can just declare and it will save gas and have the same meaning.
ConvexStakingWrapper.sol (L#54) : bool private constant _NOT_ENTERED = false;
#0 - alcueca
2022-02-02T16:15:20Z
Duplicate of #56
17.3157 USDC - $17.32
robee
Using != 0 is slightly cheaper than > 0. see https://github.com/code-423n4/2021-12-maple-findings/issues/75 for similar issue.
ConvexStakingWrapper.sol, 221: change '_supply > 0' to '_supply != 0' ConvexStakingWrapper.sol, 165: change '_supply > 0' to '_supply != 0'
#0 - alcueca
2022-02-02T16:18:08Z
Duplicate of #57
🌟 Selected for report: Dravee
Also found by: TomFrenchBlockchain, robee
25.6528 USDC - $25.65
robee
Reading a storage variable is gas costly (SLOAD). In cases of multiple read of a storage variable in the same scope, caching the first read (i.e saving as a local variable) can save gas and decrease the overall gas uses. The following is a list of functions and the storage variables that you read twice:
Cvx3CrvOracle.sol: cvx3CrvId is read twice in _peek
#0 - iamsahu
2022-02-01T14:24:26Z
#70
🌟 Selected for report: robee
95.0104 USDC - $95.01
robee
There are places in the code (especially in for-each loops) that loads the same array element more than once. In such cases, only one array boundaries check should take place, and the rest are unnecessary. Therefore, this array element should be cached in a local variable and then be loaded again using this local variable, skipping the redundent second array boundaries check:
ConvexStakingWrapper.sol, variable name: _accounts times: 5 at: _calcRewardIntegral ConvexStakingWrapper.sol, variable name: _accounts times: 5 at: _calcCvxIntegral
#0 - devtooligan
2022-02-01T01:38:14Z
I believe this is correct
#1 - GalloDaSballo
2022-02-13T17:15:19Z
Agree with the finding, because of the boundary check, you'd want to cache the value into a temporary memory variable