Sherlock-111: Wrong Inflator used in calculating HTP to determine accrualIndex #916
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
mattcushman
approved these changes
Jun 28, 2023
| @@ -874,8 +874,8 @@ contract ERC721PoolCollateralTest is ERC721HelperContract { | |||
| kickMomp: 0.000000099836282890 * 1e18, | |||
| totalBondEscrowed: 5.907892673985352325 * 1e18, | |||
| auctionPrice: 0.000004621809202112 * 1e18, | |||
| debtInAuction: 439.681348088864224563 * 1e18, | |||
| thresholdPrice: 385.774399985858744479 * 1e18, | |||
| debtInAuction: 467.777790958346588935 * 1e18, | |||
There was a problem hiding this comment.
this surprised me at first, but makes sense as less deposit was available in higher priced buckets to swap for collateral
grandizzy
changed the title
grandizzy
commented
Jun 29, 2023
| ); | ||
| } | ||
| (, uint256 maxThresholdPrice,) = _pool.loansInfo(); | ||
| maxThresholdPrice = Maths.wdiv(maxThresholdPrice, inflator); |
There was a problem hiding this comment.
hm, I don't think we need to wdiv and then wmul again to obtain HTP, _pool.loansInfo() already does this
function loansInfo() external view override returns (address, uint256, uint256) {
return (
Loans.getMax(loans).borrower,
Maths.wmul(Loans.getMax(loans).thresholdPrice, inflatorState.inflator),
Loans.noOfLoans(loans)
);
}, so I suggest getting rid of (uint256 inflator, ) = _pool.inflatorInfo(); and simplify this to
// get TP of worst loan and poolDebt
(, uint256 htp,) = _pool.loansInfo();
(, uint256 poolDebt, , ) = _pool.debtInfo();
uint256 accrualIndex;
if (htp > MAX_PRICE) accrualIndex = 1; // if HTP is over the highest price bucket then no buckets earn interest
else if (htp < MIN_PRICE) accrualIndex = MAX_FENWICK_INDEX; // if HTP is under the lowest price bucket then all buckets earn interest
else accrualIndex = _poolInfo.priceToIndex(htp);
EdNoepel
reviewed
Jun 30, 2023
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.
Description of change
High level
interestEarningDepositcalculation.Contract size (same as pre change)
Pre Change
N/A
Post Change
N/A
Gas usage (same as pre change)
Pre Change
N/A
Post Change
N/A