Open code423n4 opened 2 years ago
Great report by this warden, he should get extra points.
- State variables only set in the constructor should be declared immutable
Duplicate of https://github.com/code-423n4/2022-04-pooltogether-findings/issues/1
- internal functions only called once can be inlined to save gas
We prefer to keep the internal function _sharesToToken
since it's a pretty important one with several lines of code in it.
Duplicate of https://github.com/code-423n4/2022-04-pooltogether-findings/issues/17
- Using > 0 costs more gas than != 0 when used on a uint in a require() statement
- Don't use SafeMath once the solidity version is 0.8.0 or greater
Duplicate of https://github.com/code-423n4/2022-04-pooltogether-findings/issues/11
- Usage of uints/ints smaller than 32 bytes (256 bits) incurs overhead
We prefer to cast them first instead to have to downcast them in each functions.
- require() or revert() statements that check input arguments should be at the top of the function
PR: https://github.com/pooltogether/aave-v3-yield-source/pull/12
- Use custom errors rather than revert()/require() strings to save gas
Duplicate of https://github.com/code-423n4/2022-04-pooltogether-findings/issues/13
- Functions guaranteed to revert when called by normal users can be marked payable
Interesting but since these functions don't accept native assets, we shouldn't mark them as payable
- Method IDs can be fiddled with to reduce gas costs
Seems overly complicate to save 12 gas.
1. State variables only set in the constructor should be declared
immutable
Avoids a Gsset (20000 gas) in the constructor, and replaces each Gwarmacces (100 gas) with a
PUSH32
(3 gas)https://github.com/pooltogether/aave-v3-yield-source/blob/e63d1b0e396a5bce89f093630c282ca1c6627e44/contracts/AaveV3YieldSource.sol#L127
https://github.com/pooltogether/aave-v3-yield-source/blob/e63d1b0e396a5bce89f093630c282ca1c6627e44/contracts/AaveV3YieldSource.sol#L130
https://github.com/pooltogether/aave-v3-yield-source/blob/e63d1b0e396a5bce89f093630c282ca1c6627e44/contracts/AaveV3YieldSource.sol#L133
2.
internal
functions only called once can be inlined to save gasNot inlining costs 20 to 40 gas because of two extra
JUMP
instructions and additional stack operations needed for function calls.https://github.com/pooltogether/aave-v3-yield-source/blob/e63d1b0e396a5bce89f093630c282ca1c6627e44/contracts/AaveV3YieldSource.sol#L369
https://github.com/pooltogether/aave-v3-yield-source/blob/e63d1b0e396a5bce89f093630c282ca1c6627e44/contracts/AaveV3YieldSource.sol#L388
3. Using
> 0
costs more gas than!= 0
when used on auint
in arequire()
statementThis change saves 6 gas per instance
https://github.com/pooltogether/aave-v3-yield-source/blob/e63d1b0e396a5bce89f093630c282ca1c6627e44/contracts/AaveV3YieldSource.sol#L179
https://github.com/pooltogether/aave-v3-yield-source/blob/e63d1b0e396a5bce89f093630c282ca1c6627e44/contracts/AaveV3YieldSource.sol#L233
4. 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
https://github.com/pooltogether/aave-v3-yield-source/blob/e63d1b0e396a5bce89f093630c282ca1c6627e44/contracts/AaveV3YieldSource.sol#L47
https://github.com/pooltogether/aave-v3-yield-source/blob/e63d1b0e396a5bce89f093630c282ca1c6627e44/contracts/AaveV3YieldSource.sol#L136
https://github.com/pooltogether/aave-v3-yield-source/blob/e63d1b0e396a5bce89f093630c282ca1c6627e44/contracts/AaveV3YieldSource.sol#L145
https://github.com/pooltogether/aave-v3-yield-source/blob/e63d1b0e396a5bce89f093630c282ca1c6627e44/contracts/AaveV3YieldSource.sol#L165
5. Don't use
SafeMath
once the solidity version is 0.8.0 or greaterVersion 0.8.0 introduces internal overflow checks, so using
SafeMath
is redundant and adds overheadhttps://github.com/pooltogether/aave-v3-yield-source/blob/e63d1b0e396a5bce89f093630c282ca1c6627e44/contracts/AaveV3YieldSource.sol#L262
https://github.com/pooltogether/aave-v3-yield-source/blob/e63d1b0e396a5bce89f093630c282ca1c6627e44/contracts/AaveV3YieldSource.sol#L361
https://github.com/pooltogether/aave-v3-yield-source/blob/e63d1b0e396a5bce89f093630c282ca1c6627e44/contracts/AaveV3YieldSource.sol#L373
6.
require()
orrevert()
statements that check input arguments should be at the top of the functionChecks that involve constants should come before checks that involve state variables
https://github.com/pooltogether/aave-v3-yield-source/blob/e63d1b0e396a5bce89f093630c282ca1c6627e44/contracts/AaveV3YieldSource.sol#L177
https://github.com/pooltogether/aave-v3-yield-source/blob/e63d1b0e396a5bce89f093630c282ca1c6627e44/contracts/AaveV3YieldSource.sol#L179
7. Use custom errors rather than
revert()
/require()
strings to save gashttps://github.com/pooltogether/aave-v3-yield-source/blob/e63d1b0e396a5bce89f093630c282ca1c6627e44/contracts/AaveV3YieldSource.sol#L168
https://github.com/pooltogether/aave-v3-yield-source/blob/e63d1b0e396a5bce89f093630c282ca1c6627e44/contracts/AaveV3YieldSource.sol#L171
https://github.com/pooltogether/aave-v3-yield-source/blob/e63d1b0e396a5bce89f093630c282ca1c6627e44/contracts/AaveV3YieldSource.sol#L174
https://github.com/pooltogether/aave-v3-yield-source/blob/e63d1b0e396a5bce89f093630c282ca1c6627e44/contracts/AaveV3YieldSource.sol#L177
https://github.com/pooltogether/aave-v3-yield-source/blob/e63d1b0e396a5bce89f093630c282ca1c6627e44/contracts/AaveV3YieldSource.sol#L179
https://github.com/pooltogether/aave-v3-yield-source/blob/e63d1b0e396a5bce89f093630c282ca1c6627e44/contracts/AaveV3YieldSource.sol#L233
https://github.com/pooltogether/aave-v3-yield-source/blob/e63d1b0e396a5bce89f093630c282ca1c6627e44/contracts/AaveV3YieldSource.sol#L276
https://github.com/pooltogether/aave-v3-yield-source/blob/e63d1b0e396a5bce89f093630c282ca1c6627e44/contracts/AaveV3YieldSource.sol#L337
https://github.com/pooltogether/aave-v3-yield-source/blob/e63d1b0e396a5bce89f093630c282ca1c6627e44/contracts/AaveV3YieldSource.sol#L349
8. Functions guaranteed to revert when called by normal users can be marked
payable
If a function modifier such as
onlyOwner
is used, the function will revert if a normal user tries to pay the function. Marking the function aspayable
will lower the gas cost for legitimate callers because the compiler will not include checks for whether a payment was provided. The extra opcodes avoided areCALLVALUE
(2),DUP1
(3),ISZERO
(3),PUSH2
(3),JUMPI
(10),PUSH1
(3),DUP1
(3),REVERT
(0),JUMPDEST
(1),POP
(2), which costs an average of about 21 gas per call to the function, in addition to the extra deployment costhttps://github.com/pooltogether/aave-v3-yield-source/blob/e63d1b0e396a5bce89f093630c282ca1c6627e44/contracts/AaveV3YieldSource.sol#L275
https://github.com/pooltogether/aave-v3-yield-source/blob/e63d1b0e396a5bce89f093630c282ca1c6627e44/contracts/AaveV3YieldSource.sol#L296-L300
https://github.com/pooltogether/aave-v3-yield-source/blob/e63d1b0e396a5bce89f093630c282ca1c6627e44/contracts/AaveV3YieldSource.sol#L315-L319
https://github.com/pooltogether/aave-v3-yield-source/blob/e63d1b0e396a5bce89f093630c282ca1c6627e44/contracts/AaveV3YieldSource.sol#L332-L336
9. Method IDs can be fiddled with to reduce gas costs
See this page for details
https://github.com/pooltogether/aave-v3-yield-source/blob/e63d1b0e396a5bce89f093630c282ca1c6627e44/contracts/AaveV3YieldSource.sol