Revert if encumbered collateral is calculated as zero for non zero debt #673
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
Contributor
grandizzy
commented
Mar 8, 2023
grandizzy
commented
- encumbered collateral can be calculated as zero when tiny debt and a big LUP (encumbered = debt / LUP)
- in such case borrower could pull collateral leaving pool in a state where borrower has debt but not reflected in loans heap or auction queue
- fix scenario by reverting when encumbered collateral is 0 but debt is greater than 0
- encumbered collateral can be calculated as zero when tiny debt and a big LUP (encumbered = debt / LUP) - in such case borrower could pull collateral leaving pool in a state where borrower has debt but not reflected in loans heap or auction queue - fix scenario by reverting when encumbered collateral is 0 but debt is greater than 0
grandizzy
requested review from
EdNoepel,
ith-harvey,
mattcushman and
MikeHathaway
ith-harvey
reviewed
Mar 8, 2023
| function testPullBorrowerWithDebtCollateralEncumberedCalculatedAsZero() external { | ||
| address actor = makeAddr("actor"); | ||
|
|
||
| _mintCollateralAndApproveTokens(actor, 1000000000 * 1e18); |
There was a problem hiding this comment.
instead of creating a new actor here shouldn't we just give _lender more quote and collateral?
Keeps things tidy?
ith-harvey
reviewed
Mar 8, 2023
| _mintQuoteAndApproveTokens(actor, 1000000000000 * 1e18); | ||
|
|
||
| changePrank(actor); | ||
| _pool.addQuoteToken(200, 2572, block.timestamp + 100); |
There was a problem hiding this comment.
should we do an addInitialLiquidity call?
ith-harvey
reviewed
Mar 8, 2023
|
|
||
| changePrank(actor); | ||
| _pool.addQuoteToken(200, 2572, block.timestamp + 100); | ||
| ERC20Pool(address(_pool)).drawDebt(actor, 100, 7388, 1); |
ith-harvey
reviewed
Mar 8, 2023
| changePrank(actor); | ||
| _pool.addQuoteToken(200, 2572, block.timestamp + 100); | ||
| ERC20Pool(address(_pool)).drawDebt(actor, 100, 7388, 1); | ||
|
|
There was a problem hiding this comment.
suggest we add an _assertPool here
ith-harvey
reviewed
Mar 8, 2023
| ERC20Pool(address(_pool)).repayDebt(actor, 0, 1, actor, 7388); | ||
|
|
||
| // borrower should be able to repay and pull collateral | ||
| ERC20Pool(address(_pool)).repayDebt(actor, 120, 1, actor, 7388); |
There was a problem hiding this comment.
should do a _assertPool after action as well IMO
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.