-
Notifications
You must be signed in to change notification settings - Fork 10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor ActivePool interest tracker calls into one mintAggInterest call #93
Conversation
d558ff1
to
4265a00
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have one doubt (see inline). Other than that, this looks really nice!
@@ -596,14 +611,9 @@ contract BorrowerOperations is LiquityBase, Ownable, CheckContract, IBorrowerOpe | |||
internal | |||
{ | |||
if (_isDebtIncrease) { | |||
_activePool.increaseRecordedDebtSum(_boldChange + _accruedTroveInterest); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we change the comments in the header now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good call. And we also actually don't need the _accruedTroveInterest
param now.
@@ -866,9 +869,7 @@ contract TroveManager is ERC721, LiquityBase, Ownable, CheckContract, ITroveMana | |||
* Any surplus ETH left in the trove, is sent to the Coll surplus pool, and can be later claimed by the borrower. | |||
*/ | |||
function _redeemCloseTrove(ContractsCache memory _contractsCache, uint256 _troveId, uint _bold, uint _ETH) internal { | |||
_contractsCache.boldToken.burn(gasPoolAddress, _bold); | |||
// Update Active Pool Bold, and send ETH to account | |||
_contractsCache.activePool.decreaseRecordedDebtSum(_bold); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess we are aggregating this outside the loop in the external function redeemCollateral
? But I can find its replacement.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, this is now handled in the redemption interest PR:
#101
@@ -209,8 +209,10 @@ contract InterestRateAggregate is DevTestSetup { | |||
priceFeed.setPrice(2000e18); | |||
assertEq(activePool.aggRecordedDebt(), 0); | |||
|
|||
console.log("here 1"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this works maybe we can remove this now ;-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
haha, yes!
No description provided.