Platform: Code4rena
Start Date: 23/09/2021
Pot Size: $50,000 USDC
Total HM: 5
Participants: 14
Period: 7 days
Judge: ghoulsol
Total Solo HM: 3
Id: 32
League: ETH
Rank: 5/14
Findings: 2
Award: $2,032.62
π Selected for report: 2
π Solo Findings: 0
pauliax
function _timeRateToBlockRateit contains an expression with 3 divs that could lead to rounding errors / loss of precision: return _uint / 365 / 86400 * BLOCK_TIME / 1e18; There are a lot of constant values that do not change. These can be precalculated and re-used.
An example improvement: uint public constant DENOMINATOR = 365 * 86400 * 1e18; function _timeRateToBlockRate(uint _uint) private view returns(uint) { return _uint * BLOCK_TIME / DENOMINATOR; } This will also reduce the gas usage (fewer calculations) and improve the precision as now you will have 1 instead of 3 separate divisions unless you expect that this _uint of rate could be so high that the division is needed before the multiplication for it to not overflow.
#0 - talegift
2021-10-01T13:32:34Z
Duplicate #36
π Selected for report: pauliax
116.225 USDC - $116.23
pauliax
There are unused imports. They will increase the size of deployment with no real benefit. Consider removing unused imports to save some gas. Examples of such imports are: import './interfaces/IERC20.sol'; in InterestRateModel import './interfaces/IERC20.sol'; in LPTokenMaster import 'uniswap/uniswap-v3-core@1.0.0/contracts/libraries/FixedPoint128.sol'; and import 'uniswap/uniswap-v3-core@1.0.0/contracts/libraries/FullMath.sol'; and import './external/PositionKey.sol'; in UniswapV3Helper
Consider removing unused imports mentioned above.
#0 - talegift
2021-09-30T04:22:07Z
All of the imports mentioned in UniswapV3Helper
are being used.
The rest is correct. Will remove them.
π Selected for report: pauliax
1321.6472 USDC - $1,321.65
pauliax
The response from the ILinkOracle always assumes 8 decimals: return latestAnswer * 1e10; but itβs never actually checked if the response has 8 decimals using .decimals() function. At some point, the governor might change the number of decimals leading to an incorrect handle of the prices. At this point this is a very theoretical issue but a similar issue in a previous contest was assigned a severity of LOW: https://github.com/code-423n4/2021-04-maple-findings/issues/83 Recommend checking wethOracle.decimals() == 8.
Consider checking if the response has 8 decimals using .decimals() function.
#0 - talegift
2021-10-01T06:30:08Z
Checking this on every call would increase the gas costs. It's also very unlikely to ever change as it would break many more protocols than just ours.
This is a similar kind of risk as trusting that some upgradable proxy of a token we support won't change.
But looking at the code again, we might possibly remove dependence on Chainlink completely by denominating all token prices in ETH instead of USD so this won't be an issue.