Fix issues uncovered during Sherlock Audit #224
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
haydenshively
changed the title
Changes to gas cost
🧾 Summary (10% most significant diffs)
Full diff report 👇
|
haydenshively
force-pushed
the
sherlock-audit-fixes
branch
3 times, most recently
from
b93fb94 to
ddcbe73
Compare
haydenshively
commented
Dec 1, 2023
| (cache.borrowBase * cache.borrowIndex) / BORROWS_SCALER, | ||
| newTotalSupply | ||
| ); | ||
| return (uint72(cache.borrowIndex), inventory, inventory - cache.lastBalance, newTotalSupply); |
There was a problem hiding this comment.
inventory - cache.lastBalance == cache.borrowBase * cache.borrowIndex / BORROWS_SCALER because that's how inventory is computed in the first place.
haydenshively
commented
Dec 1, 2023
Comment on lines
-339
to
-341
| // Guard against reentrancy | ||
| require(cache.lastAccrualTime != 0, "Aloe: locked"); | ||
|
|
There was a problem hiding this comment.
Since we removed flash, there's no longer any possibility of reentrancy in Lender, so there's no reason for this guard.
haydenshively
commented
Dec 1, 2023
| newInventory, | ||
| newInventory - (newInventory - oldInventory) / rf | ||
| ), | ||
| type(uint112).max |
There was a problem hiding this comment.
We're clamping to the maximum value so that accrueInterest doesn't revert, guaranteeing that governance can always set a new rate model. Added in #206
haydenshively
commented
Dec 1, 2023
| @@ -59,6 +55,8 @@ contract Lender is Ledger { | |||
| /// @notice Sets the `rateModel` and `reserveFactor`. Only the `FACTORY` can call this. | |||
| function setRateModelAndReserveFactor(IRateModel rateModel_, uint8 reserveFactor_) external { | |||
| require(msg.sender == address(FACTORY) && reserveFactor_ > 0); | |||
|
|
|||
| accrueInterest(); | |||
haydenshively
commented
Dec 1, 2023
Comment on lines
+117
to
+122
| // Callers are free to set their own courier, but they need permission to mess with others' | ||
| if (msg.sender != beneficiary) { | ||
| require(allowance[beneficiary][msg.sender] > 0, "Aloe: courier"); | ||
| allowance[beneficiary][msg.sender] = 0; | ||
| } | ||
|
|
haydenshively
commented
Dec 1, 2023
Comment on lines
-217
to
+227
| uint256 b = borrows[msg.sender]; | ||
| require(b != 0, "Aloe: not a borrower"); | ||
|
|
||
| // Accrue interest and update reserves | ||
| (Cache memory cache, ) = _load(); | ||
|
|
||
| unchecked { | ||
| uint256 b = borrows[msg.sender]; | ||
| require(b != 0, "Aloe: not a borrower"); | ||
|
|
There was a problem hiding this comment.
Moved in as b wasn't used in the outer scope.
haydenshively
commented
Dec 1, 2023
| @@ -327,6 +309,7 @@ contract Lender is Ledger { | |||
| } | |||
|
|
|||
| function transfer(address to, uint256 shares) external returns (bool) { | |||
| accrueInterest(); | |||
There was a problem hiding this comment.
Ensures that Rewards tracking for RESERVE is correct.
haydenshively
commented
Dec 1, 2023
| } else { | ||
| (, int24 currentTick, uint16 observationIndex, uint16 observationCardinality, , , ) = pool.slot0(); |
There was a problem hiding this comment.
Moved in because it wasn't used in outer scope
haydenshively
commented
Dec 1, 2023
Comment on lines
+39
to
+42
| { | ||
| (, , , bool initialized) = pool.observations((observationIndex + 1) % observationCardinality); | ||
| if (!initialized) observationCardinality = observationIndex + 1; | ||
| } |
haydenshively
commented
Dec 1, 2023
| // Uniswap divides before multiplying, so we do too | ||
| return ( | ||
| tickCumL + ((tickCumR - tickCumL) / int56(denom)) * int56(delta), | ||
| liqCumL + uint160(((liqCumR - liqCumL) * delta) / denom) | ||
| liqCumL + uint160((uint256(liqCumR - liqCumL) * delta) / denom) |
haydenshively
force-pushed
the
sherlock-audit-fixes
branch
from
e8c7422 to
7edebd5
Compare
* Simpler computeTickTvl method in Volatility library * Eliminate IV's dependence on slot0.sqrtPriceX96 and pool.liquidity() * Clean up Oracle.sol
* Increase FEE_GROWTH_AVG_WINDOW so it's harder to manipulate * Allow IV to increase faster than it decreases to match CEX more closely * Interpolate between old and new IV samples instead of jumping * Switch to EMA updates for smoother IV
* Evaluate shortfall at the probe prices in BalanceSheet.isHealthy * Rename SLOT0_MASK_UNLEASH * Replace strain with closeFactor in bips * Only reset warning state if healthy after liquidate() * Operate on unleashTime inside liquidate() instead of pre-adding in warn() * Implement dutch auction liquidations * Reuse assets and liabilities values for gas efficiency * Clamp liquidators' amounts to what's actually available
haydenshively
force-pushed
the
sherlock-audit-fixes
branch
from
7edebd5 to
4ff9aea
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This branch contains fixes for all issues uncovered during our October Sherlock Audit. Constituent PRs' descriptions explain the changes and link to the issues they fix. The only thing for which that's not really true is liquidations, which have been further modified/updated in this PR, building on top of #223.